Opened 16 years ago

Closed 5 years ago

Last modified 4 years ago

#8760 closed Cleanup/optimization (fixed)

forms.ModelMultipleChoiceField should use "invalid_list" as error message key

Reported by: durdinator Owned by: David Smith
Component: Forms Version: dev
Severity: Normal Keywords: forms
Cc: 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

The MultipleChoiceField uses "invalid_list", but ModelMultipleChoiceField uses "list" as the key for the similar error message.

Attachments (1)

8760.diff​ (2.8 KB ) - added by Tim Graham 9 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 by Jacob, 16 years ago

milestone: 1.0 → post-1.0

It's a slight inconsistency, sure, but they are two different fields, so different error message keys are reasonable. Pushing post-1.0.

comment:2 by (none), 16 years ago

milestone: post-1.0

Milestone post-1.0 deleted

comment:3 by Jacob, 16 years ago

Triage Stage: Unreviewed → Accepted

comment:4 by Luke Plant, 14 years ago

Severity: → Normal
Type: → Cleanup/optimization

comment:5 by Aymeric Augustin, 13 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:6 by Aymeric Augustin, 13 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:7 by Markus Amalthea Magnuson, 9 years ago

Should this change happen? The comments are ambiguous :)

comment:8 by Tim Graham, 9 years ago

Has patch: set

The implementation isn't difficult (see attached patch). The question is whether or not we accept this as a backwards incompatible change (to be documented if so) or try for some deprecation path. I'm not sure how a deprecation would work.

by Tim Graham, 9 years ago

Attachment: 8760.diff​ added

comment:9 by Tim Graham, 9 years ago

Patch needs improvement: set

comment:10 by David Smith, 5 years ago

Owner: changed from nobody to David Smith
Status: new → assigned

comment:11 by David Smith, 5 years ago

I've created a pull request with the patch that Tim provided. Making this change only broke one test and I've provided a fix for this.

Hopefully this can help aid a decision on if we should make this change. If we should then is it 'ok' as a breaking change or do we need to try for a deprecation path.

From just the notes above it seems like a trivial inconsistency and not one that anyone is particularly passionate about given how long the ticket has been open. Therefore if it's going to cause any issues for anyone maybe it's just best to not fix and close the ticket?

comment:12 by David Smith, 5 years ago

Patch needs improvement: unset

comment:13 by Mariusz Felisiak, 5 years ago

Needs documentation: set
Needs tests: set
Patch needs improvement: set

​PR

comment:14 by David Smith, 5 years ago

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

I've now added a deprecation warning, docs and tests to this PR. So ready for next review.

comment:15 by Mariusz Felisiak, 5 years ago

Triage Stage: Accepted → Ready for checkin

comment:16 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: → fixed
Status: assigned → closed

In ccf32ac:

Fixed #8760 -- Changed ModelMultipleChoiceField to use invalid_list as a error message key.

comment:17 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In bf770cc8:

Refs #8760 -- Removed "list" message for ModelMultipleChoiceField per deprecation timeline.

Note: See TracTickets for help on using tickets.
Back to Top