Feature streamforms

[corysutyak]

This is a first pass at an integration with https://github.com/noripyt/wagtail-flexible-forms and not currently ready to be merged.

Big Changes:

• WFF is included as a copy in our code. There code is largely left untouched apart from unregistering their FormSubmission modeladmin classes.
• Large refactor of CoderedFormPage. The functionality of CoderedFormPage is essentially the same, but a lot of the functionality has been moved into a mixin.
• Implementation of CoderedAdvancedFormPage. This page model integrates the WFF StreamFormMixin and our new CoderedFormMixin.

Little Changes:

• Changed the way CoderedFormMixin saves files to match how WFF saves files. The only real difference is that they create a directory for the session key and then save the files in that directory. It seems better that way. It won't have any negative effects on previously uploaded files.
• Added our own CoderedFormAdmin and CoderedSubmissionAdmin modeladmin classes. WFF's versions of these classes work under the assumption that you're only using WFF and can break with Wagtail forms.

TODO:

• Integrate our StreamBlocks into the StreamForm

• Add conditional logic StreamBlocks

• Add Docs

• Test Mailchimp integration with StreamForms

Things to Note:

• A handful of new migrations, one specific to WFF
• The save_to_database attribute for CoderedFormMixin is not respected by CoderedAdvancedFormPage. Due to the form keeping state by saving an in progress submission based on the session, having the save_to_database attribute turn this feature on/off would require a bit of work on WFF. I also question it's existence to begin with. I'm not sure if it's something we need as I can't think of many good reasons why someone wouldn't want to save their form data.

[corysutyak]

Integration with our blocks has been added. The attributes for conditional logic has been added as well. The conditional logic isn't hooked up yet though.

[vsalvino]

One housekeeping item of note, please include the unmodified LICENSE file from the NoriPyt repository in the wagtail_flexible_forms directory. We are legally obligated to redistribute it based on the language in the license.

[vsalvino]

So my concern here is that this will potentially cause migrations from hell once we have to switch to built-in wagtail streamforms. I'm wondering if we should wrap/extend any blocks or concrete models in wagtail_flexible_forms, that way all the migrations are pointing to our models, and we can update under the hood as necessary. I only say that because a similar thing happened when we switched from wagtail's built-in RichTextBlock to our own RichTextBlock, and I had to do a bunch of find&replace in migration files in client projects to resolve.

I'm not 100% sure if it will cause problems or not, just trying to be proactive. Thoughts?

[corysutyak]

CoderedAdvancedFormPage uses the extended CoderedFormStepBlock which inherits from their FormStepBlock. All of our File Fields are using extended versions of their file fields. The only thing that we might want to do related to this concern is to only use wff as a library and extend their StreamFormMixin and Step/Steps classes in the CMS so there is no direct contact between the CMS and wff.

[vsalvino]

I'm thinking HTML_STREAMBLOCKS would be a better idea here instead of CONTENT_STREAMBLOCKS, to avoid making the form's streamfield too massive. I don't think it would be good to intermingle form inputs with deeply nested content blocks.

[vsalvino]

I don't really like defining classes here in this __init__ file, does not seem consistent with our structure. Do you think these classes could be moved into their own file or into the advanced_form_blocks file?

[corysutyak]

We would get a circular dependency error if it was in advanced_form_blocks.py as we need the Content/HTML block list to build the CoderedAdvancedFormStepBlock. Alternatively, we can define CoderedAdvancedFormStepBlock in advanced_form_blocks.py but we would have to repeat the Content/HTML block list in that file.

[vsalvino]

Please use docstring to document what this function does, and what the arguments are.

[vsalvino]

Please add docstring to describe what this method does.

[vsalvino]

It's not very DRY-ish that we are doing the string replace here, and also in the data_to_dict method. Is there a way to improve?

[vsalvino]

Please leave the double newline. The emails are not readable without it, because most forms have large textfields that start to run together when there is only one newline. Alternatively, it might actually help to "format" the plaintext by using a divider like \n-------------\n between the fields.

[vsalvino]

Isn't this already provided via the mixin?

[corysutyak]

Yeah, but it then gets overwritten again by the StreamFormMixin...I'll have to think of a cleaner solution.

[vsalvino]

Is this really needed here? The code is already part of coderedcms. Unless there is some kind of wagtail magic where it needs to be its own app.

[corysutyak]

As the code has their own models and hooks, it needs to be defined here. The model issue will be rectified if we treat wff as a library and extend everything in it. But we might need to repeat the code in their hooks.

[vsalvino]

I'm not entirely sure what's going on here. Does the streamform somehow store its own links to uploaded files?

[corysutyak]

https://github.com/coderedcorp/coderedcms/pull/171/files#diff-e8b7a6a93ddcb52b42325f9308a07806R400

I didn't have too much time to find the reasoning, but it appears that the above line is what causes the rendering of the fields. We're extending this model with a proxy model though, so it is fixable.

[vsalvino]

Not sure if the problem is here, but the Date/Time/Datetime blocks are not rendering with the correct html type="" attribute. They are all showing type="text" and seem to have some additional data attributes added to them. Ideally they would use the HTML5 input types to be consistent with how our current forms work.

[vsalvino]

Interestingly enough, the email field is not rendering with the correct HTML5 input type (did not test phone/tel). They are rendering as type="text". Not sure if that's a problem here, in the bootstrap4 package, or elsewhere.

[vsalvino]

Streamform field data is not exactly normalized. I know that we are normalizing file uploads to some extent, but ideally everything should be normalized to remain compatible with how the existing FormSubmission works (everything is just plain text, files are just a path). The field data within the admin email and confirmation email does not match. See email field and file fields in particular (notice the To header of confirmation email):

ADMIN EMAIL:

Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Form
From: webmaster@localhost
To: salvino@coderedcorp.com
Date: Tue, 14 May 2019 22:49:42 -0000
Message-ID: <155787418268.15924.17143601399097532929@CR-WS-5NQYFC2>

Step One Email: mailto:test2@example.com
--------------------
Step Two Name: Name
--------------------
Choices: One, Two, Three
--------------------
Date: 2019-05-07
--------------------
Date And Time: 1984-11-22T00:00:00-05:00
--------------------
File: /protected/fwcbywh4zv0vyomc0susptnw4cau68hp/ckff_wJjsOwH.txt
--------------------
Status: Complete
--------------------
User: admin
--------------------
Submit Time: 2019-05-14 22:49:18.235434+00:00
--------------------
Last Modification: 2019-05-14 22:49:42.626358+00:00
-------------------------------------------------------------------------------


CONFIRMATION EMAIL:

Content-Type: text/html; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Form
From: webmaster@localhost
To: &lt;a href=&quot;mailto:test2@example.com&quot;
 target=&quot;_blank&quot;&gt;test2@example.com&lt;/a&gt;
Date: Tue, 14 May 2019 22:49:42 -0000
Message-ID: <155787418274.15924.6862034404761348082@CR-WS-5NQYFC2>

Name: Name

File: &lt;a href=&quot;/protected/fwcbywh4zv0vyomc0susptnw4cau68hp/ckff_wJjsOwH.txt&quot; target=&quot;_blank&quot;&gt;ckff_wJjsOwH.txt&lt;/a&gt;

Date: 2019-05-07

Datetime: 1984-11-22T00:00:00-05:00

Choices: One, Two, Three
-------------------------------------------------------------------------------

To ensure rendering works correctly in plaintext, CSV, email, or any other usage, the correct normalization for email and files should be:

Email: name@example.com
File:  https://www.example.com/protected/file.txt

As a precaution, I would prefer this plain text format over doing |safe in the template, because there is a high likelihood that a spammer could attempt XSS attack and insert script into a text field.