Opened 16 years ago

Closed 16 years ago

#5923 closed (fixed)

required forms count field added into formsets

Reported by: jakub_vysoky Owned by: jkocherhans
Component: Forms Version: newforms-admin
Severity: Keywords: nfa-someday sprintdec01 edit inline add
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

We added var containing required forms count, instead of having it counted from ( total_forms - num_extra ). We needed it for javascript edit inline add.

Attachments (4)

required-forms-count-field.diff (3.0 KB ) - added by jakub_vysoky 16 years ago.
required forms count field added into formsets
help-text-edit-inline-tests.diff (21 bytes ) - added by jakub_vysoky 16 years ago.
sorry - i add this file by mistake
required-forms-count-field-tests.diff (6.1 KB ) - added by jakub_vysoky 16 years ago.
django tests to work with this ticket
required-forms-count.diff (8.1 KB ) - added by James Turnbull <james@…> 16 years ago.
Merged patch file that applies to nfa-trunk

Download all attachments as: .zip

Change History (12)

by jakub_vysoky, 16 years ago

required forms count field added into formsets

comment:1 by Tom Vergote, 16 years ago

I'm voting for the addition of this field too.

It would probably need a design decision though

comment:2 by jakub_vysoky, 16 years ago

Keywords: sprintdec01 added

by jakub_vysoky, 16 years ago

sorry - i add this file by mistake

by jakub_vysoky, 16 years ago

django tests to work with this ticket

comment:3 by jkocherhans, 16 years ago

Owner: changed from nobody to jkocherhans
Status: newassigned
Triage Stage: UnreviewedAccepted

At first glance this looks like the right solution to me, but I need to take a closer look.

comment:4 by Brian Rosner, 16 years ago

Keywords: nfa-someday added

This ticket isn't critical to the merge of newforms-admin. Tagging with nfa-someday.

comment:5 by Tom Vergote, 16 years ago

I think the patch should be applied. You may be correct that it isn't critical to the merge of nfa but the patch looks ready for checking to me, and i've tested it extensively (aka using it in a production environment) without any issue whatsoever.

Without this patch I didn't find any way to reliably insert inline forms through ajax.
This patch is fully backwards compatible, works fine, has no risk, has tests, ...

Unless you want doc's for every hidden field i don't see why you should postpone applying the patch.

comment:6 by Brian Rosner, 16 years ago

Patch needs improvement: set

Please create one patch that can be applied to the source tree. Then I will be able to bump this up to RFC.

by James Turnbull <james@…>, 16 years ago

Attachment: required-forms-count.diff added

Merged patch file that applies to nfa-trunk

comment:7 by James Turnbull <james@…>, 16 years ago

Patch needs improvement: unset

I'm also rooting for this to ticket to be resolved before merge.

Have tested and uploaded a patch file which replaces the previous two.

comment:8 by jkocherhans, 16 years ago

Resolution: fixed
Status: assignedclosed

This was fixed in [7270]. Thanks jakub_vysoky and James Turnbull.

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