Load specific submissions (sids) in one load call [#1947020]

I have some (anywhere between 1 and 1000) sids that I got from a semi-complex query (specific node type and `submitted` within a certain period) and I want to load all relevant submissions. webform_get_submissions() allows a 'sid' filter, but that's for only 1 submission. It also allows for simple filters (only equals), but not my filters. Most of webform_get_submissions() is perfect, only the 'which sids to load' part isn't.

Is there any chance you might extend webform_get_submissions() with another filter: 'sids':

  if (isset($filters['sids'])) {
    $sids = $filters['sids'];
  }

So very, very simple.

The alternative (one of my two options) is to load specific submissions separately. That means 1 - 1000 advanced queries that are all 99.9% the same. The other option is to copy webform_get_submissions() to a custom function and change a tiny part.

Even better IMO is to split the huge function into reusable parts: 1) load sids from filters, 2) load data from sids, 3) create submissions from data, 4) alter hook(s).

Comment	File	Size	Author
#2	webform-submissions-sids-filter.patch	832 bytes	pdcarto
#2

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

rudiedirkx CreditAttribution: rudiedirkx commented 19 March 2013 at 16:15

I've created webform_submission_load_multiple() which should have existed in webform.module IMO:

/**
 * Helper to load webform submission by sids.
 */
function webform_submission_load_multiple($sids) {
  $submissions = array();

  // Query the required submission data.
  $query = db_select('webform_submitted_data', 'sd');
  $query->leftJoin('webform_submissions', 's', 's.sid = sd.sid');
  $query->leftJoin('users', 'u', 'u.uid = s.uid');
  $query
    ->fields('s')
    ->fields('sd', array('cid', 'no', 'data'))
    ->fields('u', array('name'))
    ->condition('sd.sid', $sids, 'IN')
    ->orderBy('sd.sid', 'ASC')
    ->orderBy('sd.cid', 'ASC')
    ->orderBy('sd.no', 'ASC');

  $result = $query->execute();

  // Convert the queried rows into submissions.
  $previous = 0;
  foreach ($result as $row) {
    if ($row->sid != $previous) {
      $submissions[$row->sid] = new stdClass();
      $submissions[$row->sid]->sid = $row->sid;
      $submissions[$row->sid]->nid = $row->nid;
      $submissions[$row->sid]->submitted = $row->submitted;
      $submissions[$row->sid]->remote_addr = $row->remote_addr;
      $submissions[$row->sid]->uid = $row->uid;
      $submissions[$row->sid]->name = $row->name;
      $submissions[$row->sid]->is_draft = $row->is_draft;
      $submissions[$row->sid]->data = array();
    }
    // CID may be NULL if this submission does not actually contain any data.
    if ($row->cid) {
      $submissions[$row->sid]->data[$row->cid]['value'][$row->no] = $row->data;
    }
    $previous = $row->sid;
  }

  foreach (module_implements('webform_submission_load') as $module) {
    $function = $module . '_webform_submission_load';
    $function($submissions);
  }

  return $submissions;
}

Comment #2

pdcarto CreditAttribution: pdcarto commented 13 June 2013 at 18:00

Version:

7.x-3.18

» 7.x-4.x-dev

File	Size
webform-submissions-sids-filter.patch	832 bytes

I don't understand the code:

if (isset($filters['sids'])) {
    $sids = array($filters['sids']);
  }

Since it results in $sids being a array with one item, which itself is whatever $filter['sids'] was. Later, the query expects $sids to be an array of submission ids. So it only works if $filter['sids'] is a single numerical value, not an array of submission ids.

The attached patch to 7.x-4.x-dev is meant to fix this.

Comment #3

rudiedirkx CreditAttribution: rudiedirkx commented 13 June 2013 at 18:40

Where did you get

if (isset($filters['sids'])) {
    $sids = array($filters['sids']);
  }

? That's not in my code...

Also, having it in webform_get_submissions() is stupid. (I know that was my idea, but it's a bad idea.) Since webform submissions are sort-of entities, they should be loaded like entities, with a _load() and a _load_multiple() that take pk id's. webform_get_submissions() could then use that function to load and prepare submissions.

Comment #4

quicksketch

he/him

CreditAttribution: quicksketch commented 13 June 2013 at 22:17

I think @pdcarto's approach is probably the more direct solution to this problem. Webform's API predates the common _load() and _load_multiple() conventions, and adding a multi-loader option to webform_get_submissions() would only be a 3-line addition instead of duplicating the entire loader in another place.

Comment #5

rudiedirkx CreditAttribution: rudiedirkx commented 14 June 2013 at 17:15

Status:

Active

» Reviewed & tested by the community

Fair enough. Do it. How about the D8 version?

Comment #6

quicksketch

he/him

CreditAttribution: quicksketch commented 14 June 2013 at 18:55

Status:

Reviewed & tested by the community

» Needs work

Still needs a patch to be RTBC.

Do it. How about the D8 version?

I really don't know. Rewrite the whole module? D8 is too different to use any of the existing codebase, there's little reason to attempt minor consistency changes like adding a _load() and _load_multiple() functions when even those patterns are gone in D8.

Comment #7

rudiedirkx CreditAttribution: rudiedirkx commented 14 June 2013 at 22:40

There is a patch in #2...

There isn't a D8 branch yet? Webform is a definite requirement in my D8. I'd be willing, bit I'd need help (I know nothing of D8) and it wouldn't be a patch bit a rewrite. There isn't a (working prototype) D8 branch?

Comment #8

quicksketch

he/him

CreditAttribution: quicksketch commented 14 June 2013 at 23:29

There isn't a (working prototype) D8 branch?

There's not a D8 branch yet. My D8 efforts right now are concentrating on WYSIWYG in core. My plan for D8 is to (hopefully) have Webform ready within the first year of D8's release. D7 to D8 is like going from D4.7 to D7, that's how big the code changes are.

Comment #9

sonicthoughts CreditAttribution: sonicthoughts commented 15 August 2013 at 01:30

Perhaps a merge with entityforms in some magically elegant way :)

Comment #10

DanChadwick CreditAttribution: DanChadwick commented 3 November 2014 at 12:47

Issue summary:	View changes
Status:	Needs work	» Closed (won't fix)

This issue seems to have drifted into a D8 discussion. If the desire to load multiple sids still exists in the 7.x-4.x branch, please post a patch and reopen.

Comment #11

rudiedirkx CreditAttribution: rudiedirkx commented 3 November 2014 at 19:31

Seems to be 'fixed' in 4.x.

I can make a patch for 3.x. Any chance of that getting in?

Comment #12

DanChadwick CreditAttribution: DanChadwick commented 3 November 2014 at 19:35

I'm not working on 7.x-3.x. If you (or another qualified maintainer) want to take over maintenance of that branch, or 6.x for that matter, please approach quicksketch. I have a bottomless pit of 7.x-4.x issues and 8.x up-ports to deal with.

Philosophically, I'd rather see the volunteer efforts go into porting sites and webform add-on modules to 7.x-4.x, but I only control my own time.

Comment #13

rudiedirkx CreditAttribution: rudiedirkx commented 3 November 2014 at 19:39

Status:

Closed (won't fix)

» Fixed

I agree. 4.x first. I'd like to get this patch in 3.x, but not really spend every next day handling 3.x issues. Forget about 3.x. "Only bug fixes" is acceptable.

So technically, in 4.x it's fixed.