Patch adding save draft feature

riverfr0zen - February 26, 2008 - 10:36
Project:Webform
Version:5.x-2.1.1
Component:Code
Category:task
Priority:normal
Assigned:Morbus Iff
Status:patch (code needs work)
Description

This patch adds a 'save draft' feature to webforms. The feature can be enabled on a webform via the 'Advanced Settings' section when editing the webform.

If enabled, users submitting to the webform will see submit buttons to 'Save Draft' at the end of any fieldset and at the end of the form page. Choosing this button will save their results, but mark their submission to be in a 'draft' state. Currently, the user is redirected to a simple confirmation page, with a link back to the form.

When a user returns, a check is done for an existing submission in draft state. If found, the user is presented with the webform re-instated with the values from that draft.

You will need to modify the database as follows for this patch to work:

ALTER TABLE `webform_submissions` ADD `is_draft` TINYINT NOT NULL DEFAULT '0' AFTER `remote_addr` ;
ALTER TABLE `webform` ADD `allow_draft` TINYINT NOT NULL DEFAULT '0';

Hope this is useful. I made the modifications to this particular version, as that is what my client is running. However, the changes aren't too extensive, and it shouldn't be too hard to apply it to newer/in development versions.

Please do let me know if there are any issues.

AttachmentSize
save_draft.patch17.64 KB

#1

quicksketch - February 26, 2008 - 17:47

Thanks, I'll take a look at this functionality. The code just needs some minor cleanup, but we'll definitely need to write an update hook if this gets into the main version of webform. It'll also likely only be added to the 2.x version of webform, since I'm trying to shift away from 1.x which will not be ported to Drupal 6.

#2

quicksketch - March 22, 2008 - 04:47

Marked http://drupal.org/node/176598 as duplicate.

#3

Morbus Iff - March 31, 2008 - 17:10
Version:5.x-1.9» 5.x-2.x-dev
Assigned to:Anonymous» Morbus Iff

Here's a forward port to 5.x-2.x-dev, with an upgrade path.

AttachmentSize
p_226907_draft.patch25.4 KB

#4

marcingy - April 11, 2008 - 23:11

Re-roll against new version plus some small bug fixes.

AttachmentSize
p_226907_draft.patch26.38 KB

#5

simplyrichard - May 6, 2008 - 01:06

Marchingy what version is your re-roll for? It doesnt seem to work with the current one. Anyone have an updated copy of this feature? Its just plain awesome.

Richard

#6

holydrupal - May 20, 2008 - 09:09

I hope someone implement this function in to webform core in the next version.

#7

quicksketch - May 20, 2008 - 16:27

This is definitely planned for the 2.1 version of Webform. The 2.0 version had been in beta for so long, I wanted to get it out the door before adding this feature (which very well may introduce new problems). When I get some free-time I'll integrate it for the next version.

#8

quicksketch - June 7, 2008 - 20:46
Status:patch (code needs review)» patch (code needs work)

This patch has gotten out of date. A few additional comments:

- A large chunk of code in this patch is changed by unnecessary nesting of IF statements, we should put together if ($submission && user_access('access webform results')) { and if (!$is_draft) { into a single IF statement.
- Why is "#no_validate_on" needed? I don't see how this is any different from using the $_POST['op']
- If we can change both "allow_draft" and "is_draft" columns both just to "draft", that'd please my desire for consistency and short column names.

#9

riverfr0zen - June 7, 2008 - 22:40

Ha. Looks like somehow I didn't see updates to this thread. Didn't even realize forward port patches had been submitted. Haven't had a chance to look at the latest patches, so my initial comments here may not be entirely accurate - but I think they should be ok.

- I think that chunk can be flattened as you say.
- I believe I added '#no_validate_on' so that the actual op that turns off validation could be changed trivially. I was initially subverting validation through a modification to some core code, so I was a little more sensitive about it. But since I later changed it so that the validation is turned off in the module itself, I think we can change that part to $_POST['op']
- On your last point, I'm not sure I agree so much. I don't see how consistency is promoted simply because variable/column names are the same across a particular feature - in fact, it could become even more confusing if they come in proximity. Descriptive names are typically considered better than terse ones, and these aren't particularly long, in any case.

#10

armyofda12mnkeys - June 26, 2008 - 13:35
Version:5.x-2.x-dev» 5.x-2.0

Is this a Save Draft patch for the stable Recommended for 5.x version (5.x-2.0)?
Saw the last patch here is updated April 11,2008, and Webforms 5.x-2.0 was released May 09,2008 so not sure.

Thanks,
Ari

PS seems like doesnt work, last patch updates successfully but always get this warning on each page, seems like cant find webform.module and therefore display webforms i created earlier today:

* warning: include_once(./sites/all/modules/webform/webform.module) [function.include-once]: failed to open stream: Permission denied in C:\Program Files\Apache Group\Apache2\htdocs\drupaltest\includes\bootstrap.inc on line 512.
* warning: include_once() [function.include]: Failed opening './sites/all/modules/webform/webform.module' for inclusion (include_path='.;C:\dev\php\PEAR') in C:\Program Files\Apache Group\Apache2\htdocs\drupaltest\includes\bootstrap.inc on line 512.

#11

armyofda12mnkeys - June 30, 2008 - 14:18

hey marcingy,
just wondering if this patch is for the current stable release or for dev one?
didnt work with the stable branch.
Thanks for any info

#12

armyofda12mnkeys - July 22, 2008 - 21:07

Tried spending part of today trying to get the Save feature to work via the previous patch released. Looked at each change in patch file with very very few mods (comments mostly), and modified code manually for each change.
Thought i got close. but for some reason, even though $validate is off, some fields still try to validate when click on Save Draft.
Attached is what i have so far (note: i echo some variables at top just to make sure validation is off).

#13

quicksketch - July 1, 2008 - 22:59

Yikes, armyofda12mnkeys, could you contribute your changes using diff and patch?

http://drupal.org/patch/create
http://drupal.org/patch/apply

Patches allow me to easily review the changes, as well as apply them as long as the exact lines of code don't change.

#14

armyofda12mnkeys - July 23, 2008 - 04:01
Version:5.x-2.0» 5.x-2.1.1

Here is 3 patch files (remember database needs allow_draft and is_draft)

So to duplicate, make a simple form with a mandatory file (or as i observed mandatory grid component also does the trick).
I just made simple non-required yes/no quest, required file, and required textfield.
Since its a draft, the required fields shouldnt make a difference. but the upload field does complain its not filled out when saving draft.
Any idea where it invalidates the form so can add a if(!$is_draft){//then validate file/grid/others? fields}
Remember to go into the webforms Advanced Settings to Allow Draft.

I just tried to port the old code over, maybe too many changes were made for it to work.
PS dont even try to test page breaks seems like save draft breaks that currently hehe.

#15

armyofda12mnkeys - July 2, 2008 - 22:03

* btw i got forms without those certain componants to save, so let me know why those fields try to validate.

I noticed i was calling web_client_form without the 6th parameter is_draft in a few places (the three webform_menu items that call it and webform_view).
Changed those former items to
'callback arguments' => array('webform_client_form_'. $node->nid, .... , FALSE, FALSE, $submission->is_draft),
and latter to:
$form = drupal_get_form('webform_client_form_'. $node->nid, $node, $submission, $enabled, $preview, NULL, $is_draft);

(btw I also added some code to results so drafts dont get included in the Analysis, and other places in that Results tab ill get that code up later when basic stuff is more working :) )

#16

armyofda12mnkeys - July 3, 2008 - 15:05

got a bit further...

This line 'could' be set to in addition for setting all required fields set to 0, to not check the custom validate functions (ill give some reasons why this is not perfect later)...
if (!$validate) {//if validate is false, set not required
$parent_fieldset[$component['form_key']]['#required'] = 0; //change this so loops over the $parent_fieldset[$component['form_key']] for required fields to set to 0
unset( $parent_fieldset[$component['form_key']]['#validate']); //unset other required functions here; however would be nice to still validate certain things, like a file is valid. more info later
unset($form['#redirect']);//guess this does something good :)
}

This will 'work' for the upload field but for the grid componant, it has a standard required field, but in a array level below $parent_fieldset[$component['form_key']]. so you could instead of that 1st check, loop over the array for the required field and unset it as i mentioned in the //comment above.

So then file uploads and grid componants wont be checked for validation, however for file fields....
they can upload then invalid files, like if i change the value of the file field to c:/somefakefile.txt it will create this fake file on server.
To get passed this, possibly change function names of these kind of validations, like _webform_ALWAYS_validate_file so that we can check the callback name for that function set in #validate for _ALWAYS_ and see if that validation function should really be unset at this stage.
and another side effect of this is when they get back to the form, a hidden field is not set to the current filename.
This would be useful as when you now submit the form, if a hidden field is set to a filename already, there should be code to validate against that file, and not the blank file field.

Anyway, I'm messing with it, will post if get anything meaningful. Hardest part i think is adding the hidden form field validation for already uploaded files (from saving before).

#17

armyofda12mnkeys - July 18, 2008 - 20:24

Almost there I think: I set a function to recursively get rid of required fields if saving (good for grid componants which are nested), and also if saving look through validate to unset all functions except those which have a 'ALWAYSCHECK_' in function name (like file existance should still be validated but not requiredness so you need this new way to validate).

Only thing now is to change the validation so covers all those darn saved filename all scenarios when resaving or submitting....
EDIT:

<?php
if(saving)
{
    if(
is there a file already uploaded for this question?)
    {
       
delete old file
        upload
new file
        validate
new file (since saving should only do the ALWAYSCHECK_ functions)
    }
    else if
uploading brand new file or no file which will result in no error if required
   
{
       
validate new file (since saving should only do the ALWAYSCHECK_ functions)
       
upload new file
   
}
    else if
didnt pick a file
   
do nothing, dont worry bout it until u save.

}
else
//submitting
{
    if(
is there a file already saved/uploaded for this question?)
    {   
        if(
submitting with that same file saved from before, so basically didnt choose a file via Browse)
        {
           
validate old file (since saving should do all the function validate checks now )
        }
        else
//resubmitting new file
       
{
           
delete old file
            upload
new file
            validate
new file (since saving should do all the function validate checks )
        }

    }
    else if
uploading brand new file
   
{
       
validate new file (since saving should do all the function validate checks )
       
upload new file
   
}
    else
didnt pick a file
    validate file
, will error by validation checks if required field.

}
?>

There is other code in there too, like validate file for some reason acts very funky when submit non-system url path, like pretend accientally typed in file textfield 'asdf' instead of 'C:/real_file.txt' via browsing for file. So instead of 'return;' in that file validate function, I think u need to do a check in there.
Like in IE5 it wont even submit surprisingly (buttons dont work unless valid filename lol), and in FF, it will not validate bad files, and saves (sometimes i think creating a 0kb junk file on server which i also check for).

I dont have time to do a diff since my cygwin diff and even my linux diff just copies all the old module, and then all the new module. My last diff i did manually for each file via a Win program called ExamDiff. Could this be related to windows line breaks basically saying the whole new file is different since each line ends with windows linebreak and old file has unix newline? looks like this:
-whole webform.module file start
-etc
-whole webform.module file end
+new webform.module file start
+etc
+new webform.module file end

Testing this on 5.2.1.1 btw.

Then testing and seeing if can get multi-step forms to work with Save Draft feature.

#18

quicksketch - July 4, 2008 - 22:17

Yes, Windows will cause funkiness with line endings. Make sure you use a text editor that is smart enough to understand line endings and saves the new document in the same format at the original. Webform uses all UNIX line endings (\n).

#19

armyofda12mnkeys - July 19, 2008 - 22:50

Hey,
i think this works unlike previous example i uploaded, but didnt test for Multiple page forms which prob will break this (but file uploads seem iffy already with mult page forms as i remember). Anyway I'll test that part tomorrow.

I messed with old save patch and got it working to where i want it. Also changed so Required file uploads get a '*' too via some javascript since file doesnt using drupal's #required validation and its own validation functions. also when saving, #required is unset so drupal doesnt error on us, but to do this, if there is an error like file validation fails on a save(bad filename or extension), then page wont have the '*' for any field, so i also added some javascript to add that just in case that scenario occurs.

Its pretty HACKISH since i dont know the whole module and sometimes how to pass things around. One feature i added which might not be usable is where all file downloads goto $nid/$sid but like i think the way i did it was ugly so probably convert code back to regular way in _submit_file() and _insert. aka go back to old way of naming files in generic /webform folder.

I added some stuff so user editing/view his own webform can look at the file he submitted (before after you upload it, and save/submit form, you cant look at the file you submitted and only those with 'access webform results' can, booo), worked really nice with file uploads in drupal set to private.

I also added code to results so drafts are accordingly seen as Drafts in Submissions and Table and not included in the Analysis results yet.

Might want to add code to Delete the current file attached to a question if you are webform admin type or editing own webform, (and if so set it back to is_draft in database possibly if the field is required since webform wouldnt then be complete and let user know with drupal_set_message ).

I dunno why but both linux and windows cygwin, and my windows diff prog, give me problems with linebreaks doing a diff patch, so can someone run a diff on both current 5.2.1.1 module folder and this zip folder attached?

if you can i can take a look and clean up the bad stuff like whitespace changes in the diff file.


this ISNT production ready!!! use at own risk! i have a feeling the naming structure of directories change will be asked to be removed from my changes.
EDIT:
in file.inc, pass change all setFileComponantError() to setFileComponantError($form_key), including the function definition, just realized the code to 'redden' file fields wasnt being called.
Also i think i had a variable $filename i didnt use in _file_download function in webform.module right after i declare the global $user;

AttachmentSize
webform_save_draft_modified-5.2.1.1a.zip209.21 KB

#20

armyofda12mnkeys - July 21, 2008 - 15:54

heres the patch. figured out I needed to run diff with --strip-trailing-cr for windows/linux line differences...
Can anyone let me know how it is?

only thing i am not sure about is _webform_submit_file/webform_insert functions and how they handle creating directories based on nid/sid, like if its gaurenteed to create unique dir when 2 people submitting at same time. can take that out and use old code if you want. rest seems fine.

if you want to know what something does in the changes, just shoot me a message here or my account

AttachmentSize
save.patch.txt62.66 KB

#21

armyofda12mnkeys - July 22, 2008 - 21:05
Category:feature request» task
Status:patch (code needs work)» patch (code needs review)

changed the status to needs review cause i dont see any activity here. anyone lol?

#22

riverfr0zen - July 23, 2008 - 16:57

(comment removed). sorry - was having a weird moment.

#23

quicksketch - July 23, 2008 - 00:30
Status:patch (code needs review)» patch (code needs work)

I'm discouraged by the path this patch has taken. The original patch was 26K, worked, and was immaculate in code style. What happened here that made it go so far off course?

I'd like to reinforce that any code developed in Drupal should follow coding standards: http://drupal.org/coding-standards. Right now I wouldn't accept this patch under any circumstances because I'd spend hours just cleaning up all the code-styling issues.

On top of the aesthetic issue, anything that feels hackish probably is worse than it looks. I'm not going to make the mistake of accepting a patch that is going to cause a plethora of bugs and reliability problems, after everyone gets in their changes I'm the one maintaining it for the foreseeable future. Please continue the efforts, but I think this patch has gotten out of control with fixes on top of fixes.

#25

armyofda12mnkeys - July 23, 2008 - 05:48

sorry im not that great of a developer, just thought id try my hand at it. I have tested it and it works for me at least, but i understand why not willing to accept patch. just thought someone could look through and improve on it.
Again sorry. it does seem hackish mostly cause file uploads are handled a bit differently so it is like you said, fixes ontop of fixes. like the drupal core bug that you have to check required-ness differently for it. ill look into the coding standards.

#26

quicksketch - July 23, 2008 - 06:05

I don't mean to be entirely discouraging. I'm still looking at the patches every time they're updated. This patch just needs a lot more work before it'll be added to the project.

#27

rjoy - August 18, 2008 - 22:02

Thanks for directing me to this post, Quicksketch. I really did search, but didn't find it on my own. Development is not my strong point, but I'm learning. I'll take a look at the patch above and see if I can help.

Joy

#28

lefnire - August 27, 2008 - 03:08

an updates since your last attachment, armyofda12mnkeys ? I'll help out in dev

#29

armyofda12mnkeys - August 28, 2008 - 19:19

lefnire, kinda still messing with original posters attachment. i think the last attachment was usable for that version.

i just started last week porting changes to drupal6 and think its functioning almost there (have a few bugs if you want to help out lefnire, private message me if you want). i do feel some of the hacky stuff is still needed and itll prob just be a patch versus included in the module. Like I'd rather the user know a file is required with javascript hack versus try to mess with drupal's core file/validation functions.

sidenotes:
i'll look over some changes i made in drupal6 and post those so maybe so the drupal5 patch can be updated. (like when admin submits previous users submission i think i accidentally used the $users->uid to update submission, when it should be the $node->uid.
maybe some functions where i pass in something like bleh($form, $form['submitted']) and 2nd param not needed.
Also in the drupal5 module, i rethinking the file_download path i think i used something like: /webform/$nid/$sid/$file.doc since i think it might be not safe for $sid when many users submit forms at once. /webform/$nid/$file is probably fine enuff.
and with drupal6 i was a bit vague when to use $form_state versus when $form available, (and $_POST :), for example i didnt see the 'op' variable in anything other than $_POST, so i just use where i saw the variables were available. Have to pick up PDD6 book to really get into drupal6 soon ).

 
 

Drupal is a registered trademark of Dries Buytaert.