hide invisible fields from email output according to visibility rules, fixes #1131

[xini]

No description provided.

[michalkleiner]

A sentence of text for the docs/changelog would be also good since some users may rely on all fields being in the export/email regardless of their display rules, so they would need to provide a custom template after this change.

[GuySartorelli]

Some users may rely on all fields being in the export/email regardless of their display rules, so they would need to provide a custom template after this change.

That's a good point. It wouldn't hurt to add a configuration option (perhaps on the EmailRecipient next to HideFormData) to allow showing all fields regardless of form visibility rules (or perhaps it should show all by default as that's the current behaviour, and then this option would allow respecting form visibility rules in the email).

[GuySartorelli]

Also, we should probably mention in a comment on this PR that this fixes #1131 so there is a link between this and the issue.

[xini]

Thanks @GuySartorelli and @michalkleiner for your feedback.

I have

• renamed the field on SubmittedFormField to Displayed
• reversed the changes to the templates to keep the current behaviour per default
• added a config flag HideInvisibleFields to EmailRecipient to only include visible fields in the generated email. Per default this flag is false making sure the current behaviour is maintained for existing users

These changes make sure that the current behaviour is maintained unless someone changes the settings on the recipient.

Where in the docs do you want me to mention this? There is no changelog file per se.

[GuySartorelli]

That's great work! Thank you.

Where in the docs do you want me to mention this?

I'd probably recommend somewhere in https://github.com/silverstripe/silverstripe-userforms/blob/5/docs/en/userguide/form-submissions.md#email-details

[xini]

Would it make sense to set the default of HideInvisibleFields on EmailRecipient to true, so that newly created recipients get the new behaviour?

[xini]

I'd probably recommend somewhere in https://github.com/silverstripe/silverstripe-userforms/blob/5/docs/en/userguide/form-submissions.md#email-details

done.

[GuySartorelli]

Would it make sense to set the default of HideInvisibleFields on EmailRecipient to true, so that newly created recipients get the new behaviour?

I don't think so. It would be a departure from behaviour users are currently expecting and may even rely on. I can imagine cases for example where there are default values (perhaps hidden because the user filling in the form didn't tick a box that lets them edit the values) in a submission that need to be passed on to someone in the business who wasn't directly involved with setting up the form.

That said I'd defer to @michalkleiner in this case.

[xini]

@GuySartorelli That's fine with me, no problem. Anything else to get this merged?

[GuySartorelli]

That will be up to @michalkleiner or one of the other core committers - looks like the tests need fixing though.

[xini]

The Behat tests fail here: https://github.com/silverstripe/silverstripe-userforms/blob/5/tests/behat/features/userforms.feature#L144

Any ideas why? I don't get it.

[xini]

The Behat tests fail here: https://github.com/silverstripe/silverstripe-userforms/blob/5/tests/behat/features/userforms.feature#L144

Any ideas why? I don't get it.

The failing test doesn't seem to be caused by my changes, https://github.com/silverstripe/silverstripe-userforms/pull/1101/checks?check_run_id=5492750488 is failing too.

Can this be merged and tagged please?

[michalkleiner]

Yep, it seems the failure is not related to these changes.
@xini, can you please squash the commits into one with the commit message of "ENH Add config to hide invisible fields from email output". I'll merge it afterwards. Thanks!

[xini]

@michalkleiner done, thanks!

[xini]

@michalkleiner great, thanks so much!