Opened 9 years ago

Closed 9 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: UI/UX:

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

Download all attachments as: .zip

Change History (12)

Changed 9 years ago by jakub_vysoky

required forms count field added into formsets

comment:1 Changed 9 years ago by Tom Vergote

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

It would probably need a design decision though

comment:2 Changed 9 years ago by jakub_vysoky

Keywords: sprintdec01 added

Changed 9 years ago by jakub_vysoky

sorry - i add this file by mistake

Changed 9 years ago by jakub_vysoky

django tests to work with this ticket

comment:3 Changed 9 years ago by jkocherhans

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 Changed 9 years ago by Brian Rosner

Keywords: nfa-someday added

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

comment:5 Changed 9 years ago by Tom Vergote

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 Changed 9 years ago by Brian Rosner

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.

Changed 9 years ago by James Turnbull <james@…>

Attachment: required-forms-count.diff added

Merged patch file that applies to nfa-trunk

comment:7 Changed 9 years ago by James Turnbull <james@…>

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 Changed 9 years ago by jkocherhans

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