When webform results have multiple files attached, it's cumbersome and time consuming to try and download all attached files even when using table view. Since results can be downloaded, it would be nice to also include all attached files in the export. We propose an archive option in the download tab. This will create a zip file that includes the exported CSV/TSV and any attached files.
We've implemented this functionality in the attached patch which can be applied against git branch 6.x-3.x (6.x-3.9). We would like to help get this into the next 6.x-3.x release if possible.
Comment | File | Size | Author |
---|---|---|---|
#38 | 7.x-4.x-1137348-37-webform-download_submission_files.patch | 11.57 KB | fengtan |
| |||
#3 | 7.x-3.x-1137348-3.patch | 4.29 KB | dharmatech |
#1 | 6.x-3.x-1137348.patch | 4.33 KB | dharmatech |
Issue fork webform-1137348
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #1
dharmatech CreditAttribution: dharmatech commentedPatch attached.
Comment #2
quicksketchThanks, this is a nice-looking patch. If you can roll a D7 version that'll speed things along a bit, but either way I'm a bit swamped and I'm not sure I'll get to new Webform features any time soon. To make things easier yet, this patch also needs to follow a few Drupal coding standards (I use the D7 standards for both D6 and D7 for consistency): use a space on either side of concatenation dots; put else and elseif statements on the next line; use elseif instead of else if.
Comment #3
dharmatech CreditAttribution: dharmatech commentedThanks for the quick response. Attached is the patch for 6.x and 7.x. Updated to follow code convention and is now consistent with existing code.
Comment #4
quicksketchThanks @dharmatech! These patches look great. Unfortunately I'm having a few issues after applying this patch:
- On D7, the .zip file is 0 bytes. Works great in D6 though.
- After downloading the .zip, the included file is named sample_webformcsv (where "sample webform" is the name of the node). So the dot before the extension is missing. Plus, I actually used tabs as the separator, "tsv" should be the extension, not "csv".
- The .zip option only works with the delimited text file option. If you select "Excel", it just downloads the Excel file.
This is a great patch and I'd love to include it in the project. Feel free to adjust the base class and functions if needed to accompany this new functionality.
Comment #6
dharmatech CreditAttribution: dharmatech commentedPatches attached fixes the following issues:
- D7 0 byte zip file
- Add dot for file extension
- Set proper file extension depending on export type (CSV or TSV)
- Zip file generated when Excel format is selected
The diffs were generated against git branch 6.x-3.x (6.x-3.9) and master (7.x-3.9).
Comment #7
dharmatech CreditAttribution: dharmatech commentedComment #8
Owen Barton CreditAttribution: Owen Barton commentedIt appears that this was previously committed to at least the 6.x-3.x branch (haven't checked which version or if it is 7.x also).
I am setting back to CNW though, as the code references $base_path, which is not set anywhere (shouldn't this use something like http://api.drupal.org/api/drupal/includes--file.inc/function/file_direct... in any case?):
Comment #9
quicksketchWow, you're right Owen! Looks like I might have accidentally committed this as part of another issue.
According to Git history, It looks like I did this as part of a security fix (issue #54554), and I only did it to the Drupal 6 version. We should roll back this change ASAP, it wasn't ready for prime-time yet.
Comment #10
quicksketchI've rolled back the 6.x-3.x branch with this patch. I'm sort of upset with myself for letting this one go through in such a shady manner. It was entirely unintentional, sorry to anyone who may have had troubles caused by this (or are upset by its sudden removal). We'll get this working and included in a release soon.
Comment #11
quicksketchRegarding this question:
I think we can just remove the $base_path variable entirely. The file is being manipulated in the Drupal temp directory, so using of file_directory_path() is unnecessarily. Similarly, $base_path shouldn't be needed because the temp path is already relative or absolute and doesn't need a prefix.
Comment #12
dharmatech CreditAttribution: dharmatech commentedActually you do need $base_path because the ZipArchive handler needs the full path to the files. ZipArchive doesn't have a concept of "working directory" that I could find.
Comment #13
quicksketchHmm, sounds like we need to use realpath() first then, because the tmp file directory may be either relative or absolute when defined by the user.
Comment #14
Sheldon Rampton CreditAttribution: Sheldon Rampton commentedI experienced a problem when downloading submission results as a delimited text (tab delimiter) report using the 6.x-3.14 version of the webform module. I'm documenting my experience here in case it may be helpful for debugging this feature.
The webform which gave me the issues included a field for uploading a file. Our website received more than 3,000 submission results. Upon attempting to download with the "include files" option checked, I experienced errors in the form of an invalid .zip archive with a filesize of Zero KB. I tried expanding the zip archive anyway, and it produced a file with a filesize of 4KB and an extension of ".zip.cpgz".
On the theory that webform was choking on the large number of files in our submission results, I tried downloading them in batches by using the "All submissions starting from: to (optional) end" option and entering a partial range of submission IDs. This produced fatal PHP errors if the range between the starting and ending sids was too large, but I was able to successfully download everything in 13 separate batches by experimenting with sid ranges until I found ranges that worked. I seemed to be able to download successfully if the filesize of the resulting .zip archive was 100MB or smaller.
Comment #15
quicksketchWe need to implement #1327186: Use BatchAPI to Export very large data sets to CSV/Excel to help solve this problem, though I'm surprised just 3,000 submissions caused this problem for you. What's your PHP memory_limit as reported by phpinfo()?
Comment #16
Sheldon Rampton CreditAttribution: Sheldon Rampton commentedOur PHP memory_limit is 256M. I think the issue probably has more to do with the total size of all of the uploaded files, which adds up to roughly 1 gigabyte.
Comment #17
dharmatech CreditAttribution: dharmatech commentedIn this case, is the problem w/the size of the attachments or the size of the CSV? Maybe we should be splitting the files into chunks and provide a download link rather than sending the file immediately?
Comment #18
quicksketchYeah, the issue in #1276098: Add Drush command for bulk export took this approach of writing to a tmp file over multiple requests. I'd prefer to avoid a separate download link and instead stream the file, since we'll need to protect that file somehow and automatically delete it after some period of time.
Comment #19
Owen Barton CreditAttribution: Owen Barton commentedI can't think how we would use Batch API and stream the file, since Batch API ties up the browser (with the JS/reload code), adn we can't produce large files without Batch API due to PHP timeouts. Might be possible with crazy iFrame stuff, but I think the download link approach is probably more elegant. I don't think there is a way to store the file contents in Batch API or cache tables without loading it all into RAM at some point, which is problematic, so I think a filesystem file is the way to go. The file could still live in /tmp and be streamed (using the same technique as private files, but could be without depending on it), which prevents us needing to load the whole thing into RAM, and means we know when it has been successfully downloaded, so we can clean it up.
Comment #20
dharmatech CreditAttribution: dharmatech commentedAny progress? Batch API seems a bit of a diversion unless someone can provide a patch. There have been 6 3.x releases since the original patch, it would be great to see this in an official release.
Comment #21
aaronpinero CreditAttribution: aaronpinero commentedHello all. I am looking to update to the latest D6 webform release. However, I wanted to include (if possible) the patch for downloading files uploaded as part of webform submissions. Is it possible for me to use one of the patches in this issue? If so, which one? Thanks...
Comment #22
dharmatech CreditAttribution: dharmatech commentedI doubt any of the patches will apply cleanly now. The last one I posted #6 works quite well unless you have very large attachments (upwards of 1GB based on Sheldon's experience). We are approaching a year now since first posting a patch and looking back through the comments it's a stand still because there isn't a clear direction. This makes it hard for others (including me) to continue to contribute patches for this functionality.
The download link makes the most sense to me and I think Owen walks through most of it #19.
Comment #23
dromansab CreditAttribution: dromansab commentedHi,
what about Drupal 7 version? I've tried it and it doesn't work. The system downloads an empty file...
I have to install a PHP library?
Thanks
Comment #24
marameodesignMe too. I am looking for this approach in D7 - anyone has successfully used this patch ?
Thank you
Comment #25
DanChadwick CreditAttribution: DanChadwick commentedClosing for lack of activity. In all these years, there aren't that many people that want this. With the changes in download options, this would be a bit more complicated now.
Comment #26
fengtanWe happened to need this functionality so we have implemented a patch against 7.x-4.x, based on #6.
Attached is the patch, if folks are interested.
Comment #27
Liam MorlandComment #28
fengtan@Liam Morland why did you move the issue back to
Needs work
? Is there something not working with #26 ?Comment #29
Liam MorlandThe patch failed to apply. Tests need to pass.
Comment #30
fengtan@Liam Morland oh I see, the CI system tried to apply the patch against webform-7.x-3.x whereas the patch was written for 7.x-4.x.
I have added a test for 7.x-4.x -- now it applies cleanly and all unit tests pass :) https://dispatcher.drupalci.org/job/drupal_d7/44711/console
Comment #31
Liam MorlandWe can start with an easy patch, one that just implements the new method, get_filename(). Please use single-quoted string.
Comment #32
fengtanAlright -- here is a simplified patch that just implements the new method
get_filename()
and uses single-quoted strings.Comment #34
Liam MorlandThanks. I have committed #32. This will need tests.
Comment #35
fengtanRe-rolled following the refactor (46ee271). I guess this just needs tests now.
Comment #36
fengtanHere is another patch that includes tests.
Comment #38
fengtanAnd another one with more tests...
Comment #40
noahadler CreditAttribution: noahadler commentedJust confirming that patch in #38 works for me on Drupal 7.75 and Webform 4.23, so I hope this gets merged. Looking forward to this in the next release!
Comment #41
Liam MorlandMerge request needs to be rebased.
Comment #42
fengtanRebased merge request.
Comment #43
Liam MorlandThanks.
In reviewing the code, I noticed that there is a change in the return value of webform_results_download_rows_process(). This a non-backward-compatible API change. To make this backward-compatible, there needs to be an optional parameter added which tells the function to include the attached files. The comments need to be updated to document this new functionality.
compress
should probably beinclude_files
. The description that shows will ZipArchive is not available should append to the one already there.if ($options['compress'] == 1)
can beif ($options['compress'])
.Comment #44
fengtanThanks for the review. I think I have addressed everything.
Comment #45
AJV009Would this work with s3fs?