html-forms icon indicating copy to clipboard operation
html-forms copied to clipboard

Save submissions

Open JeroenSormani opened this issue 7 years ago • 7 comments

Move the 'Save submissions' global setting to a per-form action. This is a nice improvement for the upcoming GDPR and overall flexibility of the plugin.

Includes migration to automatically add the action when the save submissions was set to 'yes'.

  • Was also busy adding unit test(s), but I don't believe things are setup yet to do DB based tests, right?
  • How do you utilize autoload? When I did a $ composer dump-autoload it changed a whole bunch of files that I didn't expect.. (so to test this PR you'd want to run that yourself)

JeroenSormani avatar Apr 06 '18 20:04 JeroenSormani

Forgot to add yesterday, but added so that the 'Save submission' action is there by default when creating a new form

JeroenSormani avatar Apr 07 '18 07:04 JeroenSormani

Hey, great work!

The only issue right now for me is that I don't think saving submissions should be an action (at least right now), for the following reasons:

  • It can only be used once (as it makes no sense to save twice). Unless we build something in the UX to make this a single-use action, I think it would be better to use a simple on/off setting for now.
  • It is a bit more important than the other actions as it has other functionality built on top of this. I've added you to the HTML Forms Premium repository where you can see some examples of this, but basically it adds a notification system, CSV export and some data management to the saved submissions. It seems easier to just check a setting and point users to that setting if they want to be able to use those features (at least for now).
  • The actions are processed in the order in which they are added. If someone allows saving submissions, I would like it to always be the first action that is processed. This becomes important when someone sends themselves an email linking to the submission in their WP admin (at which point the ID is needed), for example.

The setting started out as a global setting as the people I spoke to that requested to be able to disable the storage of submissions didn't want to control this on a per-form basis.

I see how this might be useful to some people, but I would still like to give an easy way to disable all submission storage. Mostly for developers setting up sites for their customers where the customer might end up adding a new form or modifying the setting. Perhaps a filter on the default value for the setting?

dannyvankooten avatar Apr 07 '18 07:04 dannyvankooten

With the GDPR I believe it becomes more important to be able to control which forms are stored and which aren't. I myself have some forms I'd like to store and others are fine without storage. Asking permission in lines with GDPR to store anyways would be a huge turnoff.

Did the people you spoke to give a explanation to why they want a global setting vs a per-form option? I can't imagine why someone would prefer a global setting over more control for something like this (but then again I'm all about flexibility).

The points you mention are easily solvable, at least 1 and 3, I haven't checked the 2nd yet, but don't think that would be difficult for me to do.

I would still like to give an easy way to disable all submission storage This can still be controlled, just as a per-form setting. Is there a use case that you have in mind where changing this per-form would be a PITA? (its mostly a one-time configuration, right?)

JeroenSormani avatar Apr 07 '18 09:04 JeroenSormani

Did the people you spoke to give a explanation to why they want a global setting vs a per-form option?

It's not that they wanted the global option over the per-form option per se, they just didn't want to save submissions at all. In those cases, a single setting is much easier than having to turn off submissions for each individual form.

But if I wasn't clear in my previous reply, I'm all for being able to control it on an individual form level. I just don't think it should be an action (mostly speaking from an UI level here, not technical). So a simple radio setting for each form + the hf_form_default_settings filter to disable saving submissions by default will be enough / work wonderfully here, I think.

I don't see any additional benefits of turning this into an action (technically speaking) at this point. What do you think?

dannyvankooten avatar Apr 07 '18 16:04 dannyvankooten

Ah I misunderstood as I thought "Perhaps a filter on the default value for the setting?" was referring to the global setting..

I don't think the global setting needs to stay (if thats what you mean with the "hf_form_default_settings filter to disable saving by default", it can only be confusing to have things in two places. But thats not a strong opinion from my part.

I do think it can make sense as a action and setting both. It fits well as a action, plus it gives the opportunity for more flexibility in the future. E.g. if ever there needs to be settings for the action, or maybe someone wants to store it somewhere else, external or just not in the DB. That wouldn't be a setting or anything, but for that it would make more sense to have things as a action.

Dunno where I read it, but I recently read somewhere about determining when submissions would be removed after storage (for GDPR again). This would be a example of a setting which made me go for a action over setting. Or another example is to have a setting to determine which field should(n't) be stored.

I believe Ninja Forms also shows this as a action (not that HF needs to follow that, but just saying).

Overall IMO it makes most sense as a Action as storing is just another action being done after submission.

PS. I've got some other use cases where a action would only make sense to be able to add once, so adding such feature wouldn't be overkill for just this ;-)

JeroenSormani avatar Apr 08 '18 07:04 JeroenSormani

Hey, I see what your mean but I really think that we should switch it over to an action as soon as it makes sense to do so, not any sooner because as a developer it might make sense for future flexibility.

Technically, it doesn't really matter if this is an action or a setting right now (it's just moving some code around) but as long as we don't have any additional settings, order is still important and we have yet to built the use-only-once mechanism I think we should simply start out with this being a setting. I think it's nicer / easier from a UX standpoint for now and easier to get done in general. Baby steps. 😄

I'm right with you on dropping the global setting if we add the form setting though. I meant adding a filter on the hf_form_default_settings hook can be used in place of the global setting.

dannyvankooten avatar Apr 08 '18 08:04 dannyvankooten

Personally I like to use the 'systems' that are available, Actions are a example of this. I still think its fitting as a action and making sense from a UX standpoint, even without counting the future flexibility (just a nice bonus). I don't mind implementing a priority / limitation functionality either.

I think using Actions makes more sense when there are more Actions available and they're used for more then just 'Email', e.g. they're a more integrated part of things (I already got a small list of things I'd like to add as Action).

Maybe I'm thinking too much into the future again; If someone were to extend functionality they'd (if done right) follow the plugin standards, which will mean they'd either create it as a option or setting in the action. BC wise this would only give a lot of headaches if its transferred now to a setting, and in the future to a action.

We may just have to agree to disagree on this 😅

JeroenSormani avatar Apr 09 '18 06:04 JeroenSormani