Opened 17 years ago
Closed 17 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)
Change History (12)
by , 17 years ago
Attachment: | required-forms-count-field.diff added |
---|
comment:1 by , 17 years ago
I'm voting for the addition of this field too.
It would probably need a design decision though
comment:2 by , 17 years ago
Keywords: | sprintdec01 added |
---|
by , 17 years ago
Attachment: | help-text-edit-inline-tests.diff added |
---|
sorry - i add this file by mistake
by , 17 years ago
Attachment: | required-forms-count-field-tests.diff added |
---|
django tests to work with this ticket
comment:3 by , 17 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Triage Stage: | Unreviewed → Accepted |
At first glance this looks like the right solution to me, but I need to take a closer look.
comment:4 by , 17 years ago
Keywords: | nfa-someday added |
---|
This ticket isn't critical to the merge of newforms-admin. Tagging with nfa-someday.
comment:5 by , 17 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 , 17 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 , 17 years ago
Attachment: | required-forms-count.diff added |
---|
Merged patch file that applies to nfa-trunk
comment:7 by , 17 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 , 17 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
This was fixed in [7270]. Thanks jakub_vysoky and James Turnbull.
required forms count field added into formsets