Forms: More easily overridable landing page rendering

[strindhaug]

Move the landing page rendering to an easily overridable method.
Partial fix/improvement for #2834

[lb-]

Duplicate of comment on #2834

We need a way to ensure that the form_submission instance can be passed to the context of the landing page.

As per this outstanding item in #2833

The FormSubmission instance should then be passed to the render context for the "landing page".

the render_landing_page method should also accept a parameter form_submission.

Which should be generated by updating the line:
self.process_form_submission(form)
to
form_submission = self.process_form_submission(form)

Then
return self.render_landing_page(request, *args, **kwargs)
to
return self.render_landing_page(request, form_submission=form_submission, *args, **kwargs)

Finally, the context should be updated in the render_landing_page return render eg.

 # TODO: It is much better to redirect to it
context = self.get_context(request)
context['form_submission'] = form_submission
    return render(
        request,
        self.get_landing_page_template(request),
         context
        )

[strindhaug]

I've added the form_submission to the context as suggested.
I've also removed the TODO comment about redirecting, since I'm not sure it's such a good idea anymore.

[strindhaug]

I still like the idea of having a parameter that displays the landing page without having to fill out the form would be nice for template development purposes. But this should perhaps be in addition to the real submit flow and not a part of it.

(Nevermind, i forgot about preview)

[lb-]

@strindhaug when you are editing a form page, there is a way to preview both the form and the landing page easily. Does that achieve the goal you wanted?

Screenshot attached.

This made me realise that render_landing_page will need to handle cases where there is no form_submission can you make another change to the PR so that form_submission defaults to None.

Also, you will need to update the preview code here to ensure that previews of landing page use the new render_landing_page method.

https://github.com/wagtail/wagtail/blob/master/wagtail/wagtailforms/models.py#L295

[strindhaug]

Oh, I didn't realize there was a preview mode for it, that's nice. =) Well then the parameter idea is pointless.

I've updated the serve_preview and made the form_submission optional with default value None.

[lb-]

@strindhaug are you able to put in some additional tests?

possibly a model that overrides this method (eg. add something to the context or render with a different template) - to ensure that it gets used in both normal and preview mode

and also that the default landing page contains the form_submission in the context

let me know if you need a hand

[strindhaug]

I started creating a form that redirects instead of landing page but it fails before it starts because the table is missing.
How do I actually generate migrations for the test app? I'm not supposed to write them all manually I assume...

Also, is there any deterministic data in the default form_submission that can be outputted easily in the template that doesn't require modifying the model? I tried outputting {{ form_submission.get_data.your-email }} but that fails because hyphens aren't allowed. I tried adding an additional field without space to get a simple value to output but then several other tests fail because the default form now has an extra field.

[gasman]

Hi @strindhaug - you can create migrations for the test app by running django-admin.py makemigrations --settings=wagtail.tests.settings from the Wagtail root.

[lb-]

@strindhaug as get_data() returns a dict, there are multiple ways to get the data out of that within Django templates.

This Stack Overflow question has a bunch of ways, depending on what is needed. There is no default Django tag to get values from keys in a dict though.

eg. something like:

<ul>
  {% for key, value in get_data.items %}
    <li>{{ key }}: {{ value }}</li>
  {% endfor %}
</ul>

[strindhaug]

Used the for loop you suggested. Though it's sort of meaningless thing to output, at least it's testing that the form_submission is added somewhat cleanly without having lots of html entities in the test code.

Added a similar test to the TestFormWithCustomSubmission by outputting the custom field username.
This test initially failed because the overridden process_form_submission didn't return the created form_submission. So I updated this method to do so.

[lb-]

I think this comment can be removed.

[lb-]

@strindhaug added one minor comment - just to remove a comment you added about the passing of args/kwargs, I don't think that is needed.

I will bring this up at our dev meeting today.

@tomdyson FYI

[lb-]

@strindhaug one last thing is adding some docs about this new awesome feature.

Best to put them at the end of this docs page which is built from docs/reference/contrib/forms/customisation.rst.

Items that need updating (from a quick look)

• Update section Check that a submission already exists for a user to reflect changes to serve method + example should not need from django.shortcuts import render
• Update section *Multi-step form to reflect changes to serve method + example should not need from django.shortcuts import render
• Add example about overriding the render_landing_page (the one from the tests where you do a redirect is great)
• Update the paragraph that talks about form_page_landing.html is a regular Wagtail template, on the forms/index.html docs page to add note that the form_submission instance is available in the template context as ...

Let me know if you need a hand.

[lb-]

@gasman I have committed these changes to master, struggled a bit with the rebase though. The commits are not linked to this PR correctly but they are there, should I close this PR now?

[gasman]

@lb- Yes please. Having to close PRs manually is normal behaviour when you rebase them on the command line.

[lb-]

Merged