Adding `num_min` parameter break order of orderable related object during creation only

[ChinaraIsabaeva]

Issue Summary

During creation only, adding min_num parameter to InlinePanel breaks the order of orderable related object when extra forms are added by clicking "Add {obj_name}" button.

Steps to Reproduce

1. Set min_num for InlinePanel for related links to any desirable number(e.g 2) in BlogPage class

from /wagtaildemo/demo/models.py lines: 423-429

BlogPage.content_panels = [
    FieldPanel('title', classname="full title"),
    FieldPanel('date'),
    StreamFieldPanel('body'),
    InlinePanel('carousel_items', label="Carousel items"),
    InlinePanel('related_links', label="Related links", min_num=2),
]

2. In admin interface create a new "Blog page"
3. Fill in 2 minimum required forms for related links (for visibility I titled links: 'Link 1', 'Link 2' etc)
4. Click 'add related links' button to add extra form
5. Fill in the extra form
6. Repeat steps 4 and 5 N-times
7. Save draft of the page
8. After saving you can see that the order of related links is incorrect: extra links (Link 3 - Link N) become first links in the list and Link 1 and Link 2 moved to the bottom.

It seems that adding min_num parameter breaks javascript. If min_num parameter is not set, the value attr of hidden input 'ORDER' for the first form in the formset is adding. However, when I add min_num parameter, the value attr of 'ORDER' for the first N required forms in the formset, is missing (is not adding), however for extra forms, the value of 'ORDER' is adding.

Technical details

Tried in 2 environments:

First

• Python 3.4.3
• Django==1.10.8
• wagtail==1.12.2
• Browser version: Chrome Version 61.0.3163.79 (Official Build) (64-bit)

  Second

• Python 2.7.12
• Django==1.11.5
• wagtail==1.11.1
• Browser version: Chrome Version 61.0.3163.79 (Official Build) (64-bit)

[lb-]

@ChinaraIsabaeva thank you for this very detailed issue.

I have been able to reproduce this issue in 1.12.2 and also 1.13.

@jjanssen or @thibaudcolas could you look at this and add some notes about how to approach fixing this.

Cause and Potential Fix

Ideally these empty items should be created with an order value set, or maybe the order value can be set when a relation is chosen.

From what I can see, the template /wagtailadmin/templates/wagtailadmin/edit_handlers/inline_panel.html uses the empty_child method on BaseInlinePanel to generate a javascript template.

I could not easily work out what happens next, but the javascript must generate the empty children html this way, this where the order value could be added.

More Detail of Issue

It appears that when min_num is set to any number after not being used OR a bigger number than what is currently in use the pre-generated 'empty' model_choice_fields are created without an order value on this input:

<input type="hidden" name="blog_person_relationship-1-ORDER" id="id_blog_person_relationship-1-ORDER"> note - no value attribute.

Then, when you select a related model from the chooser, no order is set then either.

If you add a new item after the min_num initial set, those are created with the appropriate order value.

I also tested reordering them, the orders swap, so the 'blank' order will transfer to the next one.

All of this happens before the first save, once there are enough relations to fill the min_num they fill out their order value based on what is in the db.

[thibaudcolas]

@lb- I don't know much about Streamfield's JS, won't be of much help sorry.

[lb-]

@thibaudcolas ok - this is more edit_handler js though, not really streamfield.

Thanks @jjanssen for grabbing this one.

[jjanssen]

I'm not very familiar with this section just yet, but I'll have a look at it once I round-up the jQuery upgrade.

[m1kola]

There is a variation of this issue which results into a more unpleasant experience.

When you have an InlinePanel with min_num set (in my case it's min_num=4) and an editor fails to submit a valid form (leaves one required field empty, for example) on creation, Django throws this exception:

Traceback:

File "/home/vagrant/.virtualenvs/cca/lib/python3.6/site-packages/django/core/handlers/exception.py" in inner
  41.             response = get_response(request)

File "/home/vagrant/.virtualenvs/cca/lib/python3.6/site-packages/django/core/handlers/base.py" in _get_response
  187.                 response = self.process_exception_by_middleware(e, request)

File "/home/vagrant/.virtualenvs/cca/lib/python3.6/site-packages/django/core/handlers/base.py" in _get_response
  185.                 response = wrapped_callback(request, *callback_args, **callback_kwargs)

File "/home/vagrant/.virtualenvs/cca/lib/python3.6/site-packages/django/views/decorators/cache.py" in _cache_controlled
  43.             response = viewfunc(request, *args, **kw)

File "/home/vagrant/.virtualenvs/cca/lib/python3.6/site-packages/wagtail/wagtailadmin/urls/__init__.py" in wrapper
  96.             return view_func(request, *args, **kwargs)

File "/home/vagrant/.virtualenvs/cca/lib/python3.6/site-packages/wagtail/wagtailadmin/decorators.py" in decorated_view
  31.             return view_func(request, *args, **kwargs)

File "/home/vagrant/.virtualenvs/cca/lib/python3.6/site-packages/wagtail/wagtailadmin/views/pages.py" in create
  273.             edit_handler = edit_handler_class(instance=page, form=form)

File "/home/vagrant/.virtualenvs/cca/lib/python3.6/site-packages/wagtail/wagtailadmin/edit_handlers.py" in __init__
  269.             self.children.append(child(instance=self.instance, form=self.form))

File "/home/vagrant/.virtualenvs/cca/lib/python3.6/site-packages/wagtail/wagtailadmin/edit_handlers.py" in __init__
  269.             self.children.append(child(instance=self.instance, form=self.form))

File "/home/vagrant/.virtualenvs/cca/lib/python3.6/site-packages/wagtail/wagtailadmin/edit_handlers.py" in __init__
  712.             self.children = sorted(self.children, key=lambda x: x.form.cleaned_data['ORDER'])

Exception Type: TypeError at /admin/pages/add/facilities/facilitypage/34/
Exception Value: '<' not supported between instances of 'NoneType' and 'NoneType'

So it's possible to lose previously entered content.

Can reproduce with the following versions:

• Python 3.6.3
• Django==1.11.7 and 1.11.9
• wagtail==1.13.1

[jjanssen]

It took a little bit longer than expected to get back to this. Initially I found a bit hard to reproduce but after some input from @m1kola I managed to reproduce this. When looking at the POST-request to the backend you would suggest the order itself looks ok except for the missing order value on the initial min-num values from the form.

For eg:

title:Page title
header_names-TOTAL_FORMS:3
header_names-INITIAL_FORMS:0
header_names-MIN_NUM_FORMS:2
header_names-MAX_NUM_FORMS:1000
header_names-0-name:name 1
header_names-0-id:
header_names-0-ORDER:
header_names-0-DELETE:
header_names-1-name:name 2
header_names-1-id:
header_names-1-ORDER:
header_names-1-DELETE:
header_names-2-name:name 3
header_names-2-id:
header_names-2-ORDER:3
header_names-2-DELETE

Although this looks pretty straight-forward to solve it is more of a question on what would be the best way to solve this issue exactly. From my point of view there are two ways to solve this problem.

1. Prepopulate the initial order values into the form from the backend;
2. Prepopulate the initial order values into the form from the javascript;

But to me option 2 feels like addressing too much business logic into the responsibility of the frontend. It makes sense to handle Javascript setting the order value of adding a new field or rearranging the order. But it doesn’t necessarily make sense if Javascript would be responsible for its initial values on load.

I’m wondering how others feel on how-to address this issue.

[lb-]

IMHO - I like managing this on the serverside (option 1), reasoning is it is easier to test, more explicit in what we are providing to the clientside.

[DIGIREN]

Hey everybody. I was also having this issue, and while a fix on the actual CMS end will still be required at some point, this little hacky jQuery function should be enough to get you by. Just run it on page load on the admin side. This looks for any orderables that are missing that value="" attr that the CMS does not generate when a min_num is set, and adds it.

function fixOrderables(){
    //Finding all inputs that contain -ORDER in their ID.
    $("input[id*='-ORDER").each(function(){
        if($(this).val() === ""){
            var input_id = $(this).attr("id");
            //Getting the number out of the ID and adding 1 to construct our new value.
            var input_val = parseInt(input_id.substr(input_id.indexOf("-ORDER")-1, 1))+1;
            if (parseInt($(this).val()) != input_val){
                $(this).val(input_val);
            }
        }
    });
 }