Allow additional filters to be passed to the submission results page

This patch sounds like a good idea but the current implementation doesn't make sense. When visiting the node/%webform_menu/submissions URL, you're passing a filter of "uid" => TRUE, which then equates to UID => 1 when webform_get_submissions() is called. The call to webform_get_submission_count() also needs to take into account the user whose account is actually being filtered, not the current user.

Finally, it doesn't look like you could do filters by anything other than UID because webform_get_submissions() only supports a combination of NID and UID (unless it were also updated with a similar API change). Considering webform_get_submissions() is a public API function, I'm not sure that'd be a good move.

you're passing a filter of "uid" => TRUE, which then equates to UID => 1 when webform_get_submissions() is called

This patch fixes that, but leaving as "needs work" because of the remaining feedback in #1.

Finally, it doesn't look like you could do filters by anything other than UID because webform_get_submissions() only supports a combination of NID and UID (unless it were also updated with a similar API change). Considering webform_get_submissions() is a public API function, I'm not sure that'd be a good move.

Indeed. Perhaps a better approach would be to simply tag the query and count query, allowing modules to apply additional filters via hook_query_TAG_alter(). Not sure when we'll get to it, but at some point, James or I may submit a patch on this issue for that.

Just to keep this issue active, could you describe exactly what is being accomplished here, what's the use-case? I'm tempted to won't-fix it as is, but surely there's some reason for this push?

The use-case that prompted this for Drupal Gardens is that we're a freemium service, where we provide a free tier with nearly full functionality for people who want to try the service out or run a small, simple website on it, but where we collect revenue to cover the cost of operating and developing the service from people who are getting significant value from it. One of the features for which we think it's fair and logical to require a higher plan is for receiving lots of webform submissions.

When a site is over its allotted amount of webform submissions, we continue to allow submissions to be made, but we override the various webform submission results pages (and CSV download) to only display information about the ones within the plan limit. Currently we do this by adding $sids to the filters, though we may at some point add a column to the {webform_submissions} table.

That's our use-case, but I'm hoping the solution isn't limited to just that, but helps provide for other possible use-cases people might have in wanting to customize webform functionality.

Anyway, I didn't come up with a logical implementation for a hook_query_TAG_alter() approach, but I came up with this patch, that retains backwards compatibility. What do you think?

This patch also changes webform_get_submissions() from prefixing the uid filter with "u." to "ws.", because if no $header is passed, the {users} table isn't joined.

Rerolled patch #6 to apply to 7.x-3.13

Rerolled for 7.x-3.15. Removed the u. -> ws. change from #6, because #1309862: webform_get_submissions() called with both UID and NID may cause a PDO Exception fixed that. Note that #503264: Limit the total number of submissions implemented submission limiting logic, which is awesome, but this patch still has its original use-case of allowing submissions after some business-logic soft-limit (implemented via some other module) has been reached, but filtering them out from reports, as well as possibly other use-cases.

@effulgentsia - I may be missing (at least part of) your point in #5, but would your use case not be served by a module implementing hook_webform_submission_load() instead? You should be able to handle all your required filtering and sifting there.

It looks like some changes to webform_results_download() were lost between #6 and #7, so not all parts of the patch were working correctly. The attached version restored the ability to apply extra filters to the downloads.

In response to @vernond's comment in #9, we can't use hook_webform_submission_load() because that only gets called on webform_get_submissions(), but not on webform_get_submission_count(). So we could affect the list of submissions itself (by removing submissions after they're loaded, which would work fine although it's slightly wasteful), but the webform code which relies on the number of submissions would not work correctly.

However, reading over this issue it sounds like we should seriously consider the hook_query_TAG_alter() solution discussed in #3... It does seem that would be a simpler, cleaner change compared to adding this $extra_filters parameter in a whole bunch of places.

I've committed #1229176: Make webform_get_submission_count() support hook_query_alter() which adds a tag and alter_hook to webform_submission_count(). I agree that doing the same for webform_get_submissions() would seem like a fairly clean approach, though I'm not opposed to the approach in this issue either (other than the code ugliness it introduces).

This needed a reroll to adapt to various changes...

After looking over these changes and letting it brew for a long time, I've come to the conclusion that the approach taken here just isn't right. After this is supposed to be the entire reason why we have query tags and DBTNG to alter these queries. Especially as in all places this is merely adding $extra_filters all over the place, it seems like a use-case that is well suited to using a query alter.

I added additional query tags in #2022901: Tag webform_get_submissions() queries to facilitate this situation, but the general approach in this issue I don't think is appropriate for the module. There are a few lingering parts that could be useful, like making webform_get_submission_count() take an array of filters instead of $nid and $uid (making it match webform_get_submissions()), but generally I don't think it's a requirement at all if the query were altered instead of modifying the original caller.

If you guys disagree strongly about this, please reopen the issue.