I've only just realised that the "Save draft"button is available only for authenticated users, which is a shame! If you could make it possible to have a functioning "Save draft" button available for anonymous users as well, that would be great.

CommentFileSizeAuthor
#135 webform-anonymous-drafts-1932560-134.patch12.7 KBskylord
#133 webform-anonymous-drafts-1932560-133.patch12.67 KBskylord
#131 webform-anonymous-drafts-1932560-131.patch8.23 KBlambic
#123 webform-anonymous-drafts-1932560-123.patch8.25 KBartematem
#120 webform-anonymous-drafts-1932560-120.patch7.54 KBmingsong
#116 webform-anonymous-drafts-1932560-116.patch13.41 KBkvnm
#110 webform-anonymous-drafts-1932560-109.patch13.85 KBbezlash@gmail.com
#107 interdiff-1932560-106.txt2.71 KBmikedotexe
#106 webform-anonymous-drafts-1932560-106.patch14.63 KBmikedotexe
#104 webform-anonymous-drafts-1932560-104.patch7.69 KBmingsong
#102 webform-anonymous-drafts-1932560-102.patch818 bytesmingsong
#96 interdiff-1932560-78-96.txt3.27 KBTAT_Audaxis
#96 webform-anonymous-drafts-1932560-96.patch19.03 KBTAT_Audaxis
#95 webform-anonymous-drafts-1932560-87.patch7.46 KBmingsong
#91 webform-anonymous-drafts-1932560-87.patch7.46 KBmingsong
#87 webform-anonymous-drafts-1932560-87.patch7.86 KBmingsong
#82 webform-anonymous-drafts-1932560-82.patch5.75 KBmingsong
#78 interdiff-1932560-77-78.txt561 bytesTAT_Audaxis
#78 webform-anonymous-drafts-1932560-78.patch17.55 KBTAT_Audaxis
#77 interdiff.txt792 bytesnicrodgers
#77 webform-anonymous-drafts-1932560-77.patch17.5 KBnicrodgers
#74 webform-anonymous-drafts-1932560-74.patch17.47 KBjasonawant
#71 webform-anonymous-drafts-1932560-71.patch11.46 KBjasonawant
#69 webform-1932560-save_anon_draft-69.patch15.39 KBgrom358
#68 webform-1932560-save_anon_draft-68.patch10.67 KBgrom358
#60 webform-1932560-save_anon_draft-60.patch9.96 KBnicrodgers
#53 webform-1932560-save_anon_draft-53.patch11.3 KBnicrodgers
#52 webform-1932560-save_anon_draft-52.patch11.28 KBnicrodgers
#46 webform-1932560-46.patch6.41 KBTAT_Audaxis
#43 webform-1932560-43.patch6.44 KBnicrodgers
#43 interdiff-1932560-42-43.txt997 bytesnicrodgers
#42 webform-1932560-42.patch6.26 KBTAT_Audaxis
#38 webform-1932560-38.patch5.98 KBMichael Molchanov
#18 webform-1932560-18.patch3.99 KBTriskelion
#13 webform-1932560-13.patch3.71 KBTriskelion
#4 webform-1932560-4.patch1.43 KBTriskelion

Comments

Triskelion’s picture

To expand on the request, if an anonymous user partly fills in a form, then leaves the page for whatever reason, it would be good if the work he already did was preserved, at least for the duration of his current session.

In a brief look into the webform module, I found that removing && $user != 0 condition from line 1977 of webform.module actually allowed the button to be visible and active for the anonymous user. The form was saved with a unique sid and uid=0. The problem, obviously, is the retrieval.

I think that storing either $form_state or $sid in the users $_SESSION would permit this function to work. I am willing to look into it and, possibly, develop a patch, but I would like to know any pitfalls I should look out for.

Input requested.

phil.neale’s picture

Any answers most welcome

We are a hopefully community website . Wanting anon user to submit an article and save for later completion.
Many of our members little pc knowhow.

like to see a button >save >browse>my pc >save or website save and somehow anon user retrieves. But how to do that if anon user has no sign in

kr

Phil
Ps just on local host xamp right now drupal 7.2 Ck editor. trying to build. Stuck on this crucial issue

phil.neale’s picture

thanks triskelion

As you say the problem is retrieval.
could a work around be

Create submission form with required field user email address and generic password " anon"

Can the unique email address create the unique identity to save to ? Hopefully to their pc via browse.

Stuck on this. Might not be seeing the woods from the trees?

background:
Scenario is " community site>Want a "get involved" tab. "tell us about your wishes/hopes/what you have achieved, with photo "
kind regards

Phil

Triskelion’s picture

Status: Needs work » Needs review
StatusFileSize
new1.43 KB

I have included a patch as a starting point. This works and allows the anonymous user to save and retreive a draft, working with $_SESSIONS. The problem, as I see it, is limiting the visitor to a single submission. More work will be required.

I also noted that $_SESSIONS[''webform_submission'][$sid] = $nid. Would it not make more sense to reverse those keys ($_SESSIONS[''webform_submission'][$nid] = $sid)? It would certainly be more straightforward to limit submissions based on nid.

I am looking for feedback from the maintainers.

phil.neale’s picture

Hello triskeilion. Thankyou for post with a hope .

Is it really just the two of us raising this .

Trawled around other drupal websites with a " get involved" menu. They all ask to contact a person. None offer a direct submit "your views , about you, for direct moderation by editors.
Now come to realise a user cannot submit their article by email in a favourite word type processor that could be simply uploaded by editor via wisywig without losing a lot of formatting and images. ( LOL)
The wisywig editors available need a learning curve from users and we cannot allow them to save and continue easily ( oh)

Found this. Any use ? Add we are hardly on this scale and I dont envisage a flood gate of submissions

2. The other options is to use a system that is built-in to Drupal. Kevin has designed one for ride shares, which you can see here:

http://mwsocialforum.org/rideshare

"This would be nicely integrated into drupal, however, if we want to approve every submission and, in the weeks approaching the Forum we get flooded with posts, we may have a lag between people submitting a request or offer and it being posted to the site. The best Drupal can do is send an email to everyone in a group asking them to approve a post. That means everyone on the team will get dozens of emails per day".

Thank you for offering a hope . Do let me know if maintainers are interested and if I can help .Hung up on this would really like to get round it and started. If interim is force log in via submission form so they can save and reclaim is there any tutorial to see how to manage this process and the many odd log in's and permissions. I am trying to help myself and reading "Oreilly" etc.

kind regards

Phil

new123456789’s picture

Thank you triskelion.

Will the patch be included in the next version of the module? It's just that applying patches is all a bit too foreign to me...

Triskelion’s picture

@new123456789 - This patch is NOT ready for prime time. It is only a starting point, and I am hoping that one of the maintainers will get back to me with comments and suggestions. Webforms is a complex module, and it would be too easy to break something by advising anyone to try to apply this patch.

quicksketch’s picture

Hi guys, thanks for filing this request (though there is probably an existing request for this already, I suspect this is a duplicate). The patch approach looks like an interesting start. An obvious requirement of such a system would be that a PHP Session would need to be started for any user that fills out the webform. This could be dangerous for sites with any reasonable amount of traffic, since starting a session usually means that all other caching stops (Drupal Cache, Varnish, Akamai, etc). Of course it depends on how you set your site up, but starting a session commonly has an effect on caches.

One thing I noticed about this patch is that it would be dependent upon anonymous users already having the "access own webform submissions" permission, because otherwise that variable is never set at all.

@triskelion: Even in its current form, it looks like this patch might actually work. Is that the case?

Triskelion’s picture

@quicksketch - The patch works. (Tested before posting.) The problem that I see is limiting the anonymous user to a single draft, and expiring that draft if it is abandonned. I also want to make sure that any flood control required will be honored for the anonymous user. Still a little work to do, and thiis is where I need some guidance.

Sessions are only set for anonymous users (for multi-page forms), and are dependent on the "access own webform submissions" permissions in the existing module. That is why I preserved the condition for retrieval. Saving and retrieving drafts for logged in users would not have to change. I also think if this permission is important that the 'Save draft' button should also require it.

You will notice the comment about setting $_SESSIONS['webform_submissions'][$nid] = $sid. The reason is the array_shift required to grab the sid. If the anonymous user saved a draft for two separate webforms, shifting $_SESSIONS['webform_submissions'][$sid]would not be able to distinguish between the two, whereas reading the value of $_SESSIONS['webform_submissions'][$nid] would.

Suggestions and comments are appreciated.

quicksketch’s picture

The reason is the array_shift required to grab the sid. If the anonymous user saved a draft for two separate webforms, shifting $_SESSIONS['webform_submissions'][$sid]would not be able to distinguish between the two, whereas reading the value of $_SESSIONS['webform_submissions'][$nid] would.

Hm, I think we should be able to check if a user already has an SID that matches the current node. The value of $_SESSIONS['webform_submissions'][$sid] contains the NID. We should be able to loop through the submissions and find one that applies to the current NID. Additionally, because only the last submission can be in draft mode (if the user made multiple submissions, the previous ones would have had to be finished already), we can find the last SID for the current NID, load it, and see if it's in the draft state.

phil.neale’s picture

Dear all
I so wish I had the know how to to help resolve . But I dont. Just thankyou for taking this topic forward.
Quick remind trying to keep this topic alive but it relates to a general need too.

If anyone has created a working menu item " get involved" = submit an article for news / volunteer for support/ etc love to know how you did it.

kind regards to
triskelion, quicksketch,new123456789, for response

new123456789 ? Are we sharing same difficulty non expert ?

Phil

new123456789’s picture

Definitely a non-expert here :(

Triskelion’s picture

StatusFileSize
new3.71 KB

This patch appears to work well for the requirement in this issue.

Since $_SESSIONS['webform_submissions'] is only used for the anonymous user, I have made the change to its keying I mentioned in #9. This should have no impact on the function of the module other than simplifying the code necessary to distinguish multiple drafts. If two separate forms both have the 'Save Draft' functionaliy enabled, the anonymous user can switch back and forth between them working on both drafts at her leisure.

I have also eliminated the "access own webform submissions" requirement for anonymous draft retrieval. I assume if the developer has enabled the 'Save Draft' button for a web form, this should be sufficient. If not, I would suggest a new permission "Allow save draft" which could be activated by role. This would avoid the confusion I encountered when initially trying to get this to work, until I realized the anonymous user required this permission.

phil.neale’s picture

Hello 1234

Wondering if in same boat: This issue is sparked because I cannot make email notification work via xamp local host.
Just want to test my site as I go and try to learn. Prepared to compromise and force anon user to register and so interact with our site if thats the best we can do for now.
Cant make email to new user sign on work at all . My platform is windows win 7

Can you ?

Fear windows users are second class in drupal. If you have the same underlying issue can we be allys to promote a solution.?

kind regards

Phil

quicksketch’s picture

Since $_SESSIONS['webform_submissions'] is only used for the anonymous user, I have made the change to its keying I mentioned in #9. This should have no impact on the function of the module other than simplifying the code necessary to distinguish multiple drafts.

Hm, except for the unknown impact on other custom and contrib modules. There's no hard requirement for needing to flip the keys. As mentioned in #10, you should be able to work work with the existing structure, though it's not quite as convenient.

I'm not sure what to do about adding a new permission (or not). This could be made a per-form setting too, converting the existing checkbox into radio buttons:

Drafts:
(•) Do not allow drafts
( ) Allow drafts for logged in users
( ) Allow drafts for all users
Drafts allow a user to save a form and finish it later.

Thoughts?

Triskelion’s picture

True, I hadn't considered custom modules. Not a good idea to break someone else's work :-(

As for the new permission, I think Occam's razor should apply: keep it simple unless there is demand for the complex. If a developer is going to allow anonymous input, and allow drafts, why would they prevent anonymous drafts. I would leave that for a future feature request.

However I have turned up a potential problem with this patch. After I confirmed that the anonymous user had the ability to save and retrieve a draft, I later logged in and returned to the webform. It appears that the draft I saved as anonymous was picked up when I was logged in from the same machine. (Not sure, but will test. It might have been a draft I don't remember saving as admin.) I want to make sure that there is no unwanted interaction between $_SESSION and use of cookies before I give this a green light for user review.

new123456789’s picture

Unfortunately I am a drupal newbie and have never applied a patch before. Looks very complicated :(

Triskelion’s picture

StatusFileSize
new3.99 KB

Set up on a fresh install and put it through its paces. Prior concern about cookies not an issue: they were separate drafts.

I did turn up a couple of things I had to address. $_SESSION may not be set for the anonymous user on first pass and has to be checked. Also the $_SESSION variable has to be unset after form submission or the draft persists.

I have also restored the key=>value order in $_SESSIONS['webform_submissions'] to its legacy order.

liam morland’s picture

If an anon user starts a submission, resulting in a saved draft, and then abandons the session, does the draft get erased? I could imagine a mess of old drafts building up if this does not happen.

Triskelion’s picture

Webforms does not implement hook_cron. The module has always relied on administration to clear submissions. Perhaps you should create another issue as a feature request.

liam morland’s picture

I bring this up here because this patch introduces the possibility of these unowned drafts building up. Previously, the worst that could happen in this regard is that every user on your site has one draft saved. With this patch, there is no limit to how many old drafts could be hanging around.

Triskelion’s picture

That would be one draft for each webform for each user. I think your point is a valid one, but it is going to require a stated policy for draft, or submission expiration, and an implementation of hook_cron to accomplish it. In that sense, it is outside the scope of this issue.

Bring it up as a new issue, and reference this one as part of your explanation of the requirements. I actually think it would be a good feature even if drafts are restricted to authenticated users, particularly on high traffic sites with a large user base.

diwant’s picture

Triskelion, have you looked at the lazy registration module? It's at http://drupal.org/project/lazy_registration and it tries to deal with this same issue of persisting anonymous sessions to newly registered accounts.

pat redmond’s picture

I'm not really sure whether this is a new issue, or part of this issue.

I want to display the 'Save Draft' button for anonymous users, however when they click the button I want them to be given the option to sign in or sign up.

When I've tested, it seems that if a registered user is NOT logged in and they start completing the form then they won't get the 'Save Form' button. If they navigate to the log in page, they log in, then return to the form all of their progress will be lost. Is this something I'm missing, or is that the module currently operates?

rv0’s picture

Patch in #18 also applies to 7.x-1.x-dev and is working great for me

One issue, if the draft is removed for whatever reason, it remains in the session and tries to load it, resulting in error or wsod depending on your config

added small test to check if the draft is there
if sure there's a better way though

      // Make sure the draft really exists
      if (!empty($draft_sid) && is_numeric($draft_sid)) {
        $res = db_query("SELECT * from {webform_submissions} WHERE is_draft = 0 AND sid = :sid", array(
          ':sid' => $draft_sid));
        $rowcount = $res->rowCount();
        if (empty($rowcount)) {
          $draft_sid = 0;
        }
      }
quicksketch’s picture

If they navigate to the log in page, they log in, then return to the form all of their progress will be lost. Is this something I'm missing, or is that the module currently operates?

Some modules like Flag will automatically migrate a user's flagged content from their cookie if they log into the site. I think that would be an excellent thing to include in this issue also.

Webforms does not implement hook_cron. The module has always relied on administration to clear submissions. Perhaps you should create another issue as a feature request.

Webform doesn't yet have a hook_cron(), but we can certainly add one. I think Liam's point is fair, we need to at least have a plan for how to handle an unlimited number of anonymous submissions piling up, even if we don't include it immediately. We don't want to work ourselves into a corner where we introduce a significant problem without any available solution.

diwant’s picture

Quicksketch, any ideas on which module we might use to help with this? There are a few about that help with saving anonymous data to an account.

Here's a good (albeit dated) layout of different approaches (doesn't include the lazy_registration one I linked to above. This article talks about the lazyreg module, a different one): http://data.agaric.com/agaric-wants-registration-flow-let-people-registe...

We should keep an eye on this one: https://drupal.org/project/flow because that's apparently a backport to 7 of how this problem is solved in D8.

vako’s picture

Wasn't the Save Draft option available for anonymous users in the 7.3 version? I believe I was using it but lost it when upgraded to the 4.0 version today.

liam morland’s picture

sonicthoughts’s picture

@quicksketch - do you see this feature getting committed? We are using webforms for a long questionnaire with anon users and would like them to be able to recover input. my big concern with drafts piling up is there is no way to bulk delete that I'm aware of.

quicksketch’s picture

Status: Needs review » Needs work

@quicksketch - do you see this feature getting committed?

Yes, but the unanswered problems raised by this issue have me concerned: 1) deleting unfinished anonymous forms 2) the WSOD issue @rv0 mentioned (but we don't really have any adequate information on that 3) migrating anonymous submissions to logged in ones.

This patch seems just a little half-done. After #1890422: Drafts are created for anonymous users when they change pages, we probably also need a reroll to ensure submissions are saved between pages now. After that issue we prevent anonymous users from saving drafts between pages. That feature should be restored in this patch if anonymous users are allowed to submit drafts.

Aracon’s picture

Issue summary: View changes

"Automatically save draft between pages" seems not to be working for anons with this patch. Or am I doing something wrong?

Aracon’s picture

Looks like the problem is in this line of code:

// Inform the submit handlers that a draft will be saved.
  $form_state['save_draft'] = $form_state['values']['op'] == $draft_op || ($node->webform['auto_save'] && !$form_state['webform_completed'] && user_is_logged_in());

I removed user_logged_in() and now drafts save automatically:

$form_state['save_draft'] = $form_state['values']['op'] == $draft_op || ($node->webform['auto_save'] && !$form_state['webform_completed']);
liam morland’s picture

liam morland’s picture

danchadwick’s picture

Status: Needs work » Closed (won't fix)

Unless someone wants to tackle bulk deletion and address the 2-year-old issues in #31, I'm going to close this. If there is continuing interest (and in particular patch creation), please to re-open.

Michael Molchanov’s picture

Status: Closed (won't fix) » Needs review
Michael Molchanov’s picture

StatusFileSize
new5.98 KB

The last submitted patch, 4: webform-1932560-4.patch, failed testing.

The last submitted patch, 13: webform-1932560-13.patch, failed testing.

The last submitted patch, 18: webform-1932560-18.patch, failed testing.

TAT_Audaxis’s picture

StatusFileSize
new6.26 KB

Michael,
I'd like to say a big THANK YOU for this patch, the right thing at the right time for me.
I've tested it locally, no problem, it works as expected.

I've changed few lines from your patch, to handle the case when an anonymous user begins to submit some steps of a multi-step webform, then got logged in. This way, draft can be retrieved from $_SESSION even if $user->uid doesn't match anymore. And it also changes the "owner" of the submission, from anonymous to the logged in user.

Thank you again!

nicrodgers’s picture

StatusFileSize
new997 bytes
new6.44 KB

Thanks Michael and TAT_Audaxis, the patch in 42 applied cleanly for me to the latest dev release and successfully allows anonymous users to save a draft.

I have updated the patch so that if a draft is saved anonymously, then the user logs in and returns to the form, the original submission is transferred to the logged in user account automatically when they view the form. In the version in #42 from TAT_Audaxis, the user had to click 'save draft' again for this to happen, and if they didn't, then the original submission would be inaccessible to them.

The updated patch also fixes a small typo.

Status: Needs review » Needs work

The last submitted patch, 43: webform-1932560-43.patch, failed testing.

TAT_Audaxis’s picture

StatusFileSize
new6.41 KB

Thanks for your work, Nic.
Unfortunately, all these patches fail to apply since commit a9bb592598cce33167df6468067dd11d8ea669b6.

I've fixed Nic's patch, replacing line $_SESSION['webform_submission'][$form_state['values']['details']['sid']] = $node->nid; with $_SESSION['webform_submission'][$sid] = $node->nid; (line #3315)

danchadwick’s picture

Status: Needs work » Closed (won't fix)

Just a note -- as per #31 and #36, this patch won't be committed until someone addresses how to cope with anonymous drafts which can no longer be completed because the session has expired.

Therefore I'm closing this as I don't think there is any activity on this roadblock. Please re-open if there is.

nicrodgers’s picture

Status: Closed (won't fix) » Active

Just a note -- as per #31 and #36, this patch won't be committed until someone addresses how to cope with anonymous drafts which can no longer be completed because the session has expired.

Happy to work on this. Here's what I'm thinking:

* Add a configurable variable to the global web form settings - "Keep anonymous drafts for xxxx hours"
* Use hook_cron within the webform.module to check for any anonymous drafts last modified more than XX hours ago, and delete them

Any thoughts / suggestions / improvements?

liam morland’s picture

I don't think it needs configuration: On each cron run, have it erase all drafts that are not associated with an active session.

danchadwick’s picture

I like Liam's idea. Remember that deleting submissions can be expensive. I'm thinking we may need a hidden (No UI) batch size per cron run.

Also need cache and reverse proxy testing (eg Varnish).

nicrodgers’s picture

@Liam Morland - if we use expired sessions as per your suggestion, it won't handle the use case where the session remains active but the user hasn't modified the webform submission. For example, they save draft, then continue using the site periodically but never return back to the form.

Having it as a configurable value seems more flexible to me, plus covers this use case. What are your concerns with making it configurable?

nicrodgers’s picture

StatusFileSize
new11.28 KB

Here's an initial patch (that includes everything in #46, plus configuration for expiring stale drafts, with hook_cron support) to show my thinking. Will need to add Batch API support and do extensive testing with caching as per #50

nicrodgers’s picture

StatusFileSize
new11.3 KB

Whoops, forgot to limit to just anonymous drafts. Fixed.

liam morland’s picture

Avoiding configuration keeps things simpler for administrators. I had thought that sessions last a short enough amount of time that a user is very unlikely to be on a site continuously enough that this would be a problem.

danchadwick’s picture

I think I agree with Liam. At some point, the session is times out, right?

nicrodgers’s picture

The default session cookie lifetime in D7 is 23 days (2000000 seconds, see settings.php).

On high traffic sites, you could feasibly want a long cookie lifetime (to ensure users stay logged in, for example) but not want to retain anonymous drafts for so long.

I understand your argument about complexity for admins, but by setting the default value to 7 days and placing it within 'advanced settings', I don't think that should be a big problem?

danchadwick’s picture

I understand your argument about complexity for admins, but by setting the default value to 7 days and placing it within 'advanced settings', I don't think that should be a big problem?

Yes. I don't have any enthusiasm for adding a very obscure feature to the admin interface. I don't even have much enthusiasm for making it configurable.

If someone is running a high volume site and wants a shorter time, they can write their own cron task, right?

liam morland’s picture

Making the time shorter could just be a matter of using cron to call the delete function more often than Webform does by itself. A really busy site would probably reduce the session length anyway to avoid the session table getting too big.

nicrodgers’s picture

Ok, I'll continue on that basis.

nicrodgers’s picture

Status: Active » Needs review
StatusFileSize
new9.96 KB

Here's a patch based on #46, updated to include support for cleaning up anonymous saved drafts on sessions that have expired. It uses the Queue API.

Needs testing - locally and also with Varnish / reverse proxy config.

danchadwick’s picture

#60 passes test, which doesn't mean much since the tests don't cover this functionality. No need to requeue. What it needs is reviewers -- especially those running caches and reverse proxy servers.

danchadwick’s picture

Those of you who want to see this committed need to find some reviewers with caches and reverse proxy server to test this. Otherwise this patch will probably rot on the vine here. Sad, but true.

bleeuwen’s picture

Hi,

another idea is to save the draft in a file and giving the opportunity to restore the file (populate the form from the saved file)

This option has the advantages:
- it does not matter the user is authenticated or not
- no hassle about sessions or cookies
- process can be taken up from a different computer

This options is used in other form-systems which are used in public government sites.

gregori.goossens’s picture

Hi,

i try to use #60 patch on fresh install (Drupal 7.40 and webform 4.11).

i have a problem with the "Per user submission limit" option in anonymous submission with multipage,

if we use user limit set to 1 for example, when anonymous user submit first page with save draft enabled, webform_client_form_submit function is execute (no return on line 3324) then $sid is empty so set new cookie.
Then on next page submit (for example back to page 1 button), process stop with "You have already submitted this form." message due to check done in webform_submission_user_limit_check in webform.submissions.inc (check only if cookie exist).

Maybe we could only set cookie if webform_completed ?

danchadwick’s picture

Status: Needs review » Needs work
TAT_Audaxis’s picture

Hello,

My project using WebForm has been postponed to 2016, so I can't help as much as I would like to for this patch.
The project will use Varnish Cache, however I think it will be simpler for us to disable cache for all webform pages. Especially because visitors can log into our site and because of multi-step forms, you usually never cache that kind of pages. Otherwise we should use ESI to handle cookies, but it will surely end with a lot of rare and complex use cases...
Do you have another idea to support reverse-proxy caching?

Anyway, a little comment about the patch #60 : could would please complete each watchdog() function with the appropriate debug level?
Thanks in advance, and keep it up!

grom358’s picture

StatusFileSize
new10.67 KB

I added a test case for testing saving of drafts.

grom358’s picture

StatusFileSize
new15.39 KB

Sorry forgot to include a file in the patch.

torotil’s picture

I've recently created a module named webform prefill that at least touches this use-case:

  • It saves entered values in the browsers sessionStorage.
  • It pre-fills webforms whenever they are loaded with the latest values (matched by form_key).

Depending on the life-time of the sessionStorage this already solves the "save as draft" for anonymous. Since it's handled entirely on the client-side this completely circumvents the caching issues.

jasonawant’s picture

Status: Needs work » Needs review
StatusFileSize
new11.46 KB

Hi All,

I refactored the query logic in #69 patch. I found that it did not correctly query for stale anonymous webform submissions.

The query logic now queries for both anonymous webform submissions with expired sessions and webform submissions without matching session records. I found that, depending on how the PHP garbage collection was configured, an anonymous webform submission's associated session record may or may not have already been deleted by the time cron runs.

I have also setup a site on Pantheon's platform, which uses Varnish. I'm not sure what to test here, so please let me know. From what I can tell, the anonymous webform submissions are properly deleted on cron. Also, an anonymous session is properly expired client-side and the associated saved anonymous webform submission is no longer available to the site visitor once their session is expired. I have the site configured with the following settings. http://dev-webform-test.pantheon.io/

ini_set('session.gc_probability', 1);
ini_set('session.gc_divisor', 100);
ini_set('session.gc_maxlifetime', 300);
ini_set('session.cookie_lifetime', 300);

Feedback welcome. I'm eager right now to work on this. Jason

quicksketch’s picture

Wow this looks pretty great @jasonawant and @grom358! There are a few extra spaces (or maybe tabs) in places in the code indentation, but in general this is quite clear and a significant expansion of Webform's save as draft functionality!

A couple of things

+    // Clear _SESSION for anonymous user after submission
+    if (isset($_SESSION['webform_submission'][$sid])) {
+      unset($_SESSION['webform_submission'][$sid]);
+    }

It looks like this would clear up the session variable for a submission ID, but leave the $_SESSION['webform_submission'] as an empty array value (thus keeping the session). Have you confirmed that this actually destroys the session?

Could you add in the WebformDraftTestCase file from #69 into your next patch? We may need to expand the test coverage further, I haven't read the full extend of it yet.

I tried out your demo site and things work remarkably well! That's great that just setting the session makes it so that pages are properly not cached for users filling out the form. Do we already provide a warning on the form to indicate that allowing the "Save as Draft" functionality may result in a performance impact? It seems like we should warn administrators that enabling it may cause a large number of sessions to bypass caching (especially on a multipage form where users may give up before reaching the end).

quicksketch’s picture

It looks like this would clear up the session variable for a submission ID, but leave the $_SESSION['webform_submission'] as an empty array value (thus keeping the session).

I've confirmed that the session stays active after submitting the form. We probably just need another IF statement right after that first one:

+    // Clear $_SESSION for anonymous user after submission.
+    if (isset($_SESSION['webform_submission'][$sid])) {
+      unset($_SESSION['webform_submission'][$sid]);
+    }
+    if (count($_SESSION['webform_submission']) === 0) {
+      unset($_SESSION['webform_submission']);
+    }

(Also let's make sure we end comments in periods per Drupal coding standards).

jasonawant’s picture

StatusFileSize
new17.47 KB

Thanks for reviewing this. Here's a patch that includes your recommendation in #73, code standards fixes, changes to allow_draft config help text, and the test file.

BD3’s picture

I can confirm that the patch in #74 is working for me. Thank you!

robin van emden’s picture

Tested #74 extensively for a few days now, have not encountered any problems as of yet.

nicrodgers’s picture

StatusFileSize
new17.5 KB
new792 bytes

#74 has been working well for us too, thanks for everyone's help moving this forward. I've added the severity parameter to the 2 new watchdog calls, as per TAT_Audaxis's feedback in #67.

TAT_Audaxis’s picture

Hi,

While this patch is working really great, I stumbled upon a new bug. Let me explain the scenario:

  1. An anonymous user begins to fill a form
  2. The draft is saved, in the SQL table named "webform_submissions" there is a new line with "uid = 0" and "session_id = xxxxxxx"
  3. The user goes into the login process, he is now authenticated; its session identifier is now "yyyyyyyy"
  4. He's back on the form, the SQL table "webform_submissions" has been updated with "uid = 123", but not the field "session_id"
  5. The cron runs, it discovers that the session xxxxxxx does not exist anymore, then deletes this submission draft
  6. The user submits the final step of the form
  7. Only data from final steps have been saved, everything before is lost.

Here is a simple patch to fix that case. Basically, it updates the field "session_id" when needed.

nicrodgers’s picture

Good spot, and the interdiff looks like a sensible solution to me.

gedur’s picture

Note that we've just realeased a different approach to allow draft submissions to anonymous users by a $_GET authcode: https://www.drupal.org/project/webform_draft_authlink any feedback is welcome!
Webform is awesome!!!

TAT_Audaxis’s picture

There is a bug in fix #77 :

watchdog('webform', 'Deleted a stale anonymous draft.', WATCHDOG_INFO);

It should be :
watchdog('webform', 'Deleted a stale anonymous draft.', array(), WATCHDOG_INFO);

Because of this, it triggers a fatal error in /admin/reports/dblog.

mingsong’s picture

StatusFileSize
new5.75 KB

Thank you for the solutions above.
In my case, I need to use Webform to create an anonymous multi-page survey which has about 13 pages.
As the Session performance issue, the solution from #77 and #78 is not suitable for my case.

I develop another solution which just uses a Cookie to store a draft submission ID (sid) for an anonymous user.
Following are the logics to resume a draft submission for an anonymous user:
1. Check if this is an anonymous user.
2. If so, check if there is a draft submission id (sid) stored in a Cookie.
3. If the Cookie exists, load this submission from database and resume it.

rv0’s picture

@Minsong wouldn't that allow anyone to change their cookie value to another sid and then see another users values (security problem) ?

mingsong’s picture

@ rv0
Yes, you are right. There are still some security improvement jobs need to do.
Just a quick thought, it can be worked around by encrypting the sid prior to storing in a Cookie. And then decrypting it while we read it from the Cookie.

In this way, we need a two-way encryption API plus a private key which can be specified by user.

liam morland’s picture

There is already code for a submission access token. See webform_get_submission_access_token().

mingsong’s picture

Thanks a lots @Liam Morland.
Excellent! I will make some changes for patch #82 to improve the security.

I have a question about submission permission for anonymous user.

If we assign the permission 'Access own webform submissions' to anonymous user, then any anonymous user could access any submissions submitted by an anonymous user through the following path:
/node/%node/submission/%submission

Am I right?

mingsong’s picture

StatusFileSize
new7.86 KB

New patch which improves security for patch #82

Status: Needs review » Needs work

The last submitted patch, 87: webform-anonymous-drafts-1932560-87.patch, failed testing.

mingsong’s picture

mingsong’s picture

mingsong’s picture

StatusFileSize
new7.46 KB
mingsong’s picture

liam morland’s picture

If we assign the permission 'Access own webform submissions' to anonymous user, then any anonymous user could access any submissions submitted by an anonymous user through the following path:
/node/%node/submission/%submission

Am I right?

I don't think so. Look at webform_submission_access(). Look at places where webform_get_submission_access_token() is called.

mingsong’s picture

mingsong’s picture

StatusFileSize
new7.46 KB
TAT_Audaxis’s picture

The last messages of this thread are completely different from the original patch because they explore new paths and choices.

I'm back to the roots with message #78 and those new modifications:

  • Fix watchdog missing parameter (message #81)
  • Check if the draft is really a draft, for a non-completed submission, before using it
  • Allow to disable message « A partially-completed form was found », thanks to a variable_get() flag.

This new patch has been tested and will be on a production site before next week.

navneet0693’s picture

Status: Needs work » Needs review
mbabazi’s picture

#95 worked for me and is in place on a production site now, but required a manual creation of the session_id column in the database table. Thanks for this.

nicrodgers’s picture

Taken a look at the updated patch in #96 and it looks good. Just a couple of nitpicks:

+++ b/webform.module
@@ -2043,16 +2043,23 @@ function webform_node_view($node, $view_mode) {
+        // No draft or submission already closed

Missing a full stop.

+++ b/webform.module
@@ -4242,6 +4249,9 @@ function webform_page_labels($node, $form_state = array()) {
+      $result = variable_get('webform_draft_found_message',FALSE);

Needs a space after the comma.

Also, I might be missing something, but I can't see anywhere that sets the webform_draft_found_message variable - is that intentional?

We're using our patch in #96 in our UAT environment and are due to start a round of testing next week, so I'll let you know if any problems are spotted.

TAT_Audaxis’s picture

Hi Nic,

Thank you for your feedback.

Also, I might be missing something, but I can't see anywhere that sets the webform_draft_found_message variable - is that intentional?

Yes, it was. I think this is the kind of variables you set usually in settings.php.
You can add a checkbox in the admin interface if you feels it would be better for everyone.

mingsong’s picture

Hi @mbabazi,
Thank you for your feedback.

#95 worked for me and is in place on a production site now, but required a manual creation of the session_id column in the database table. Thanks for this.

The #95 patch doesn't use Session but Cookie to store draft submission ID for anonymous users. So there is no need to create a session_id column in database table.

mingsong’s picture

StatusFileSize
new818 bytes

There is an undefined function error if per user submission limit is unlimited.
This patch #102 fixes this issue.

Status: Needs review » Needs work

The last submitted patch, 102: webform-anonymous-drafts-1932560-102.patch, failed testing.

mingsong’s picture

StatusFileSize
new7.69 KB

This patch fixes undefined function error for patch #95

mingsong’s picture

mikedotexe’s picture

StatusFileSize
new14.63 KB

We had an issue with this with a site that uses HTTPS for anonymous users.
In that case, Drupal's session table uses the column ssid instead of sid

This causes an issue where anonymous drafts are always queued for deletion via cron.
I also added some minor coding standard changes mentioned in #99 and another one I noticed.

Also, to be clear, since there are two patches with the same name, this is in regards to the patches from #78, #96, etc.

mikedotexe’s picture

StatusFileSize
new2.71 KB

Interdiff for #106
(I thought I could add another file by editing later. My bad.)

kevster’s picture

I just tried to patch using -106 against latest 7.x-4.x-dev from sept 1st and I'm getting warnings about the patch not applying cleanly - should I have used a previous patch before this one?

PHP Storm is telling me "couldnt find context for patch. Some fragments were applied at the best possible place. Please check carefully"

Many thx..

bezlash@gmail.com’s picture

Hi,

I've recreated this patch against the latest 7.x-4.x-dev since it was no longer applying.

bezlash@gmail.com’s picture

StatusFileSize
new13.85 KB
liam morland’s picture

Status: Needs work » Needs review

Setting it to "needs review" will cause it to be tested.

The last submitted patch, 106: webform-anonymous-drafts-1932560-106.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 110: webform-anonymous-drafts-1932560-109.patch, failed testing.

gravisrs’s picture

We may want webform_draft_found_message variable to be configurable in admin/config/content/webform.
Best approach would be having this to be set individually for each form.

wjhessels’s picture

This one isn't working for me. Also tried developer version with #110

kvnm’s picture

StatusFileSize
new13.41 KB

Here's a re-roll of the mega-patch in #106 that should work against 7.x-4.16 and 7.x-4.x-dev.

kvnm’s picture

Status: Needs work » Needs review
liam morland’s picture

Thanks for the patch.

It needs to be documented what webform_draft_found_message is for. Is there a reason the default value for it is not NULL? That would mean the default can be omitted from variable_get().

Please run a coding standards check on the patch. I noticed one thing: There is a space missing before "FALSE" in the line with webform_draft_found_message.

There should be tests for the new functionality.

TAT_Audaxis’s picture

I think there is a bug with anonymous session garbage collection.
In webform_cron(), the following line looks wrong:

<?php
$db_or->condition('s.timestamp', REQUEST_TIME + $cookie_info['lifetime'], '<');
?>

Because of that, every cron seems to delete every anonymous session. Following the logic of _drupal_session_garbage_collection(), the right condition should be:

<?php
$db_or->condition('s.timestamp', REQUEST_TIME - $cookie_info['lifetime'], '<');
?>

Are you OK with that change?

mingsong’s picture

StatusFileSize
new7.54 KB

Recreate patch at #104 for the version of 7.x-4.16.

If you are using the patch from #104 and just updated the Webform module to this version, you need to apply this patch to it.

-Andrew-’s picture

In general the patch from #120 seems to work perfectly fine. Although I found one issue which can be easily fixed. The issue is that the draft is not being saved within the "autosave" feature.

The issue lies in the following line (webform.module; line 3316):

// Inform the submit handlers that a draft will be saved.
$form_state['save_draft'] = in_array($form_state['values']['op'], array($draft_op, '__AUTOSAVE__')) || ($node->webform['auto_save'] && !$form_state['values']['details']['finished'] && !$form_state['webform_completed'] && user_is_logged_in());

and can be fixed by simply removing the following condition: && user_is_logged_in()

so tat is looks like this:

$form_state['save_draft'] = in_array($form_state['values']['op'], array($draft_op, '__AUTOSAVE__')) || ($node->webform['auto_save'] && !$form_state['values']['details']['finished'] && !$form_state['webform_completed']);

For me it worked and hopefully it will help some of you as well.

swhitters’s picture

Any chance of turning this into anonymous autosave behavior instead of a "save draft" button? I cannot rely on my users to save the data themselves and I'd like to have access to the data even if it is incomplete.

artematem’s picture

StatusFileSize
new8.25 KB

Applied fix from #121.

liam morland’s picture

Thanks for the patch. The new functionality needs tests.

skylord’s picture

Hm. Hasn't anyone ever compared patches in #123 and in #116? They are quite different. The one in #116 touches test suite a little, relies on Drupal sessions and not cookies and etc.

dadaisme’s picture

Patch #123 seems to work fine.

Thx!!

liam morland’s picture

@#125: I prefer the idea of using existing Drupal sessions vs. using cookies directly.

alan d.’s picture

@Liam Morland
So #110, #116 over #104, #120, #123?

mstrelan’s picture

Patch in #116 needs a reroll for 7.x-4.19 and latest 7.x-4.x-dev as webform_update_7431() is already declared.

Anonymous’s picture

I am using patch #123 succesfull.

lambic’s picture

StatusFileSize
new8.23 KB

Attaching new patch that applies to 4.21

skylord’s picture

BTW, #131 is reroll of #123. Patch #116 (which one is better as of me) applies fine to 4.21.

skylord’s picture

StatusFileSize
new12.67 KB

Found a typo in webform_cron in #116 resulting in deleting all drafts each cron run regardless of configured session timeout. :-)
So, here is a updated patched rerolled for 4.22.

Status: Needs review » Needs work

The last submitted patch, 133: webform-anonymous-drafts-1932560-133.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

skylord’s picture

Status: Needs work » Needs review
StatusFileSize
new12.7 KB

Hm. Added index check to fix testing.

liam morland’s picture

Status: Needs review » Closed (outdated)

Drupal 7 is no longer supported. If this applies to a supported version, please re-open.

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.