Hook to add additional submission information in downloads

Hi Liam,

Check out includes/webform.export.inc - there are some hooks you can use to create a customised download of results.

[edit] You'd need to add a new handler and extend the webform_exporter class in your custom module

Thanks Liam, this patch looks pretty good overall, however if this is specifically for the exports, we should include "csv" in the hook names. I know that's not technically accurate but it would match _webform_csv_headers_component, _webform_csv_data_component, and the new hooks added in #1533408: Allow modules to modify exported submissions. If we added this, I'd also like to see a drupal_alter() in there. There have been multiple requests to hide or obfuscate the IP Addresses. This would be one place where users may want to exclude some columns.

I don't think these two locations are output in a way that is similar right now. The submission information on an individual submission view page is just manually printed out in the theme layer rather than being formatted into an array of properties to be printed out. Additionally I'm not sure that you would always want the same data in both places, so perhaps it would be better to keep the two places as separate hooks anyway. In this patch, I would like to see the two calls to drupal_alter() added in here. It's pretty uncommon to have a registry hook like this doesn't also include an alter.

Thanks, just ran across #1793288: How to export submitted data with seconds? which would benefit from the alter hook. I'll review when I get the chance.

+  $csv_components += module_invoke_all('webform_csv_submission_information_info');
+  // Prepend information fields with "-" to indent.
+  foreach ($csv_components as $key => &$title) {
+    if ($key !== 'info') {
+      $title = '-' . $title;
+    }
+  }

Missing the alter hook here, though it's in place the second time hook_webform_csv_submission_information_info() is called. Calls to this function should probably be wrapped in a helper to avoid indescrepencies like this.

We need documentation on both new hooks, "Implements hook_x" isn't useful documentation if there's no matching hook documentation to reference.

Going through the whole "needs review" queue now so anything that's not ready I'm moving out. :)

We don't need $sid or $row count here I don't think:

+function webform_webform_results_download_submission_information_data($item, $submission, $sid, $row_count) {

$sid is the same as $submission->sid, right? $row_count probably isn't relevant here and it isn't passed into any other functions for generating csv cells. It exists for the exporter's benefit. The cell data shouldn't need to know which cell it's in.

After simplifying the general hook, the alter hook can ditch the special $context variable and just pass in $item and $submission.

Still missing docs on these two new hooks.

Why the name change? Although I understand these aren't really just for "csv" files, hook_webform_results_download_submission_information_data() is one hella-long hook name.

Thanks again for the updates Liam. Things look pretty good.

I have documented the hooks at their implementation. Is this OK or should they be documented elsewhere?

All hooks should be documented in webform.api.php with a sample. Actual implementations in the module can just use the abbreviated PHPdoc "Implements hook_x()."

Hi Liam, sorry for the long long wait on this one. Although the hook names in this are awfully verbose, I don't want to hold up this issue any longer. Thanks for your patience and effort on pushing this forward. I've committed #25 as-is and it'll be in tomorrow's release. Thanks!

+++ b/includes/webform.report.inc
@@ -869,31 +865,13 @@ function webform_results_download_rows($node, $options, $serial_start = 0) {
+    foreach (array_keys($submission_information) as $token) {
+      $cell = module_invoke_all('webform_results_download_submission_information_data', $token, $submission, $options, $serial_start, $row_count);
+      $context = array('token' => $token, 'submission' => $submission, 'options' => $options, 'serial_start' => $serial_start, 'row_count' => $row_count);
+      drupal_alter('webform_results_download_submission_information_data', $cell, $context);
+      // implode() to ensure everything from a single value goes into one column, even if more than one module responds to this item.
+      $row[] = implode(', ', $cell);
     }

@@ -954,6 +925,49 @@ function webform_results_download_submission_information($node, $options = array
+function webform_webform_results_download_submission_information_data($token, $submission, array $options, $serial_start, $row_count) {

This hook and its alter one are being triggered for every submission * the number of submission keys.

Put a few thousand submissions then webform_webform_results_download_submission_information_data() will be called many thousand times. Put another implementor with a few extra keys and an alter hook and then you will probably crash the download:)

Put a few thousand submissions then webform_webform_results_download_submission_information_data() will be called many thousand times. Put another implementor with a few extra keys and an alter hook and then you will probably crash the download:)

This is true, but largely mitigated because 1) A record of hook implementations are cached (both in memory and in cache tables), so if you don't have any modules that use that hook, there's very little overhead 2) submission downloads are batched and only a hundred or so lines are added per batch request. So no matter how big the download is, it shouldn't crash, but it will take longer.

submission downloads are batched and only a hundred or so lines are added per batch request.

well yes but the default to DOWNLOAD RANGE OPTIONS is to export all submissions and the batch size is dependent to the number of you components.
So with just one component and one more implementor which is something like that (just duplicating webform_webform_results_download_submission_information_data):

function MYMODULE_webform_results_download_submission_information_data($token, $submission, array $options, $serial_start, $row_count) {
  switch ($token) {
    case 'webform_serial2':
      return $serial_start + $row_count;
    case 'webform_sid2':
      return $submission->sid;
    case 'webform_time2':
      if (!empty($options['iso8601_date'])) {
        return format_date($submission->submitted, 'custom', 'c', 'UTC');
      }
      else {
        return format_date($submission->submitted, 'short');
      }
    case 'webform_draft2':
      return $submission->is_draft;
    case 'webform_ip_address2':
      return $submission->remote_addr;
    case 'webform_uid2':
      return $submission->uid;
    case 'webform_username2':
      return $submission->name;
  }
}

Calls my implementation about 70k times!
And if you add another 70k times calling webform_webform_results_download_submission_information_data() and 70k times my imaginary alter hook, you can see some huge amount of function calls.

It would be good to have a followup here to switch this hook to run just once per batch...or in the worse case once for every submission..because i am not convinced that adding custom keys this way scales

I am for now using the patch i posted in #2117285: Allow extra data to be added to submissions in result displays but once i find time to upgrade and port to the approach here, i will open the followup

Edit: I tried the above with a webform with 5k submissions and one email component

It would be good to have a followup here to switch this hook to run just once per batch...or in the worse case once for every submission..because i am not convinced that adding custom keys this way scales

Reducing this down to one call per row would certainly be an improvement. I'm not sure one-per batch would be easy to implement or to utilize as a developer.

Needs porting to 8.x-4.x.

Committed and pushed fb80272 to 8.x-4.x. Thanks!