Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#12398 closed (fixed)

Add `TypedMultipleChoiceField` to compliment `TypedChoiceField`.

Reported by: mrmachine Owned by: nobody
Component: Forms Version: master
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Patch includes docs and tests. This could also fix #5481.

Attachments (2)

12398-TypedMultipleChoiceField-r11901.diff (6.8 KB) - added by mrmachine 5 years ago.
12398-TypedMultipleChoiceField-r14586.diff (7.1 KB) - added by anonymous 4 years ago.

Download all attachments as: .zip

Change History (10)

Changed 5 years ago by mrmachine

comment:1 Changed 5 years ago by mrmachine

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 5 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted
  • Version SVN deleted

comment:3 Changed 4 years ago by lukeplant

  • Patch needs improvement set

Looks fairly good to me. A few things:

  • I would change the comment that starts "Hack alert", because I don't think it is a hack, and I think the comment about catching both exceptions is incorrect - it catches one and raises another.
  • The patch doesn't apply cleanly at the moment. The tests have moved.
  • The test names are strange - why do they start at 70 instead of 1?

With these addressed, feel free to move to Ready For Checkin.

Changed 4 years ago by anonymous

comment:4 Changed 4 years ago by mrmachine

  • Patch needs improvement unset
  • Triage Stage changed from Accepted to Ready for checkin
  • Version set to SVN

I've updated the patch according to your feedback. The underlying implementation changed a fair bit (to use to_python() instead of clean()), so the "hack" comment is gone as it was no longer relevant, anyway. The tests were named for consistency with the surrounding tests back then, which had a unique sequential number suffix for all tests in the module, rather than for each group of tests.

I think we can also mark #5481 as fixed or invalid (with instruction to use TypedMultipleChoiceField) now?

comment:5 Changed 4 years ago by mrmachine

  • milestone set to 1.3

comment:6 Changed 4 years ago by russellm

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

(In [14829]) Fixed #12398 -- Added a TypedMultipleChoiceField. Thanks to Tai Lee.

comment:7 Changed 4 years ago by timo

(In [15082]) Refs #12398 - TypedMultipleChoiceField is new in Django 1.3

comment:8 Changed 4 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

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