formset.ordered_forms should try to return ordered forms if is_valid() is false

[nickname123]

I am not sure if this should be a bug report or a feature request.

I think that formset.ordered_forms should be usable even if the formset is invalid. This is particularly useful in conjunction with Form Wizard for me, but I think there are plenty of other cases where this would be nice.

See ​https://github.com/django/django/blob/stable/1.6.x/django/forms/formsets.py#L216 where is_valid() == False causes an AttributeError

It would be useful for rendering templates. The particular data I am working is much easier to understand if it is displayed in order.

I was attempting to allow ordering the forms without moving to the next step in a form wizard. So I overrode the formset is_valid method like below with the intentions that the user could post ordering without moving to the next step:

class BaseOrderFormSet(BaseFormSet):
    """
        THIS FORMSET RETURNS INVALID IF THE USER SUBMITTED A REQUEST
        TO ADD ADDITIONAL FORMS OR UPDATE THE FORMSET INSTEAD OF
        ACTUALLY SUBMITTING THE DATA FOR SAVING
    """
    can_add_form = True
    can_update_formset = True
    def is_valid(self):
        # do not validate if we need to add another row
        return not ADD_FORM_KEY in self.data and not UPDATE_FORMSET_KEY in self.data and super(BaseOrderFormSet, self).is_valid()

(note FunkyBob's first comment seems out of context because we were having a conversation in django-users first but it wasn't relevant enough to the ticket to copy over and it spanned a lot of other comments)

[18:57] <gp> Why does the ordered_forms property of a formset required is_valid() to be True? https://github.com/django/django/blob/stable/1.6.x/django/forms/formsets.py#L216
[18:58] <FunkyBob> isn't that a fundamental of the FormWizard ? you can't progress until each 'form' is valid?
[18:59] <gp> FunkyBob: I am trying to allow users to order the forms in the formset but I cannot output them using ordered_forms until the formset is valid
[18:59] <gp> I don't understand why is_valid is important for outputting the forms in an ordered fashion
[19:00] <FunkyBob> I see
[19:01] <gp> My use case is the following:  I have added "add another" function like the admin.  Then I have added an "update" button that rerenders the template.  I was hoping to easily allow ordering without javascript
[19:02] <gp> Which I can still do... that is just how I ran into this
[19:04] <gp> Well by still can do I mean I can do it if I reimplement the ordered_forms property.  Didn't know if there was a specific reason this was only allowed for valid forms
[19:04] <gp> Or if maybe I should submit a bug/feature request
[19:05] <+bmispelon> gp: fwiw, the full test suite still passes if self.is_valid() is removed
[19:08] <gp> Ty.  I will submit a ticket and override it for my formset

[nickname123]

I've made a quick fix and overridden the property on a BaseFormSet subclass. It appears to work fine so far. The prefix might need to be generated more "properly". I will post it as a patch against trunk if this gets accepted

[nickname123]

It would make sense to include updating the deleted_forms property with this patch

[pjrharley]

PR here ​https://github.com/django/django/pull/2093

I tried doing the same for deleted forms but it broke a whole load of other tests that were expecting an empty list. It would be really easy to just fix the tests but it makes me wonder if anyone is relying on that behaviour? I'd guess not, so if it is ok to change I'll submit that too.

[nickname123]

I think the patch needs to do just a little more. If the form isn't valid then cleaned_data will raise an exception won't it? Unless that changed in more recent versions of django. I'm stuck on an older version with the project I am currently working with.

I got around this by falling back to using form.data. See line 33 of the file I attached. This still doesn't work for forms without data. It makes sense to me for initial forms to have an ordering as well as it might be nice for ordered forms to return all forms that aren't deleted for easy rendering in templates.

[Tim Graham]

I guess I'm not entirely sold on the concept given the cleaned_data issues as noted above, but I've never used ordered_forms before so I don't want to speak definitely on the issue.

[pjrharley]

Replying to timo:

I guess I'm not entirely sold on the concept given the cleaned_data issues as noted above, but I've never used ordered_forms before so I don't want to speak definitely on the issue.

To be honest I don't really have any opinions on it either. I've never used them, I was just looking for some tickets to work on

I'm not sure I really understand the issue though either - the test I added passes, and it accesses cleaned_data after asserting that the form isn't valid.

I'll unassign it from myself for now, but I'm happy to work on it again if there's a clear way forward.

[Jaap Roes]

Isn't this a duplicate of #14238

[Tim Graham]

Closing as duplicate of #8165.