Support from Acquia helps fund testing for Drupal Acquia logo

Comments

harrrrrrr’s picture

Version: 6.x-3.9 » 7.x-3.11
Status: Active » Reviewed & tested by the community

Tested and approved.

harrrrrrr’s picture

Version: 7.x-3.11 » 6.x-3.11
quicksketch’s picture

Title: Webfrom status views handler » Add a Views handler (field/filter/sort/etc.) for Webform "status" field
Category: support » feature
Status: Reviewed & tested by the community » Needs work
+  $data['webform']['table']['group'] = t('Webform infos');
+
+  $data['webform']['table']['join'] = array(
+    'node' => array(
+      'left_field' => 'nid',
+      'field' => 'nid',
+    ),
+  );

The name of this category should probably be just "Webform". "infos" is not a real word.

+    'title' => t('Satus'),

Should be "Status".

+      'handler' => 'views_handler_field_numeric',

We should write a custom handler for "Active" or "Inactive" to be displayed instead of 0 or 1 for the field value, just like the Node Published/Unpublished field and filter. Using the numeric handler is functional but lazy.

jweowu’s picture

Version: 6.x-3.15 » 6.x-3.11
FileSize
1.05 KB

Here's a re-roll of the original patch (edit: for 7.x-3.15). I've fixed the language issues, but haven't implemented a custom handler.

I've also changed the join to an inner join, because an outer join was unnecessary (for the query being generated).

An outer join might potentially be desirable, as no row is added to the webform table until a form component is created. However the View would need to be testing the specified value against COALESCE(webform.status, 0) or similar for a left join to actually return those rows. A NULL webform probably shouldn't be considered equivalent to a closed webform, though.

jweowu’s picture

Version: 7.x-3.15 » 6.x-3.15

edit: Oops.

jweowu’s picture

Version: 6.x-3.11 » 7.x-3.15

Ah, I had meant to set that to 7.x-3.15. I saw the 6.x-3.9 » 7.x-3.11 change in #1, didn't notice that it had been changed back in #2, and managed to just adjust the minor number without even noticing the major number. So to clarify, the original patch was for Drupal 6, and the patch in #4 is for Drupal 7.

quicksketch’s picture

Version: 6.x-3.11 » 7.x-3.15

The latest patch works on both D6 and D7 without changes.

This patch needs a dedicated handler for Open/Closed status for both the field and filter. The comment module Views integration provides a pretty good example of how to do this with minimal effort.

bdone’s picture

Version: 7.x-3.15 » 6.x-3.x-dev
Status: Needs work » Needs review
FileSize
4.13 KB

here's a patch against 6.x-3.x that includes...

  • boolean vs. numeric handlers
  • adds a dedicated field handler
  • adds a dedicated filter handler
  • formats code comments to match existing ones
bdone’s picture

Version: 6.x-3.x-dev » 7.x-3.x-dev
FileSize
8.62 KB
4.05 KB

here's the same patch against 7.x-3.x and a view export for testing

bdone’s picture

do these patches need anything else for a review?

quicksketch’s picture

Status: Needs review » Needs work
FileSize
2.18 KB

The D6 patch doesn't seem compatible with Views 2 on Drupal 6. Was this written against Views 3?

The D7 patch worked pretty well but the type (open/closed, yes/no, etc.) would always revert back to open/close when editing. I've fixed that problem in this iteration.

This patch also makes a new "Webform" group in Views, but we're currently piggy-backing off the "Node" or "Content" groups for things like the Webform Edit and Results links. I think it'd make sense to convert these all to the "Webform" group, or we can keep using the existing groups. We shouldn't split between two different locations though.

quicksketch’s picture

Doh, apparently I just forgot to add the new files to the webform/views directory. Patch worked fine in Views 2 after that minor fix. I'll look into the minor cleanup and post an updated patch.

quicksketch’s picture

Status: Needs work » Fixed
FileSize
7.46 KB
7.46 KB

Updated patches that simply move all the Webform fields into a common category.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

joshuautley’s picture

any reason the field "status" is causing inner joins? And can there be a way to change to a left join in the views UI via a reduce duplicates?

I'm looking at... webform.views.inc - line: 19 - 25 (specifically line 23)

  $data['webform']['table']['join'] = array(
    'node' => array(
      'left_field' => 'nid',
      'field' => 'nid',
      'type' => 'INNER',
    ),
  );

Returns: INNER JOIN {webform} webform ON node.nid = webform.nid

So, I changed line 23 to "LEFT" instead of "INNER" as shown below...

  $data['webform']['table']['join'] = array(
    'node' => array(
      'left_field' => 'nid',
      'field' => 'nid',
      'type' => 'LEFT',
    ),
  );

Which returns: LEFT JOIN {webform} webform ON node.nid = webform.nid

Is there any objection to why I should not do this? And if no objection can this be changed?

joshuautley’s picture

Issue summary: View changes
Status: Closed (fixed) » Needs review
jweowu’s picture

I don't remember details of this issue, but my comment #4 above is why it's an inner join in the code.

Things may well have changed in the intervening years, so it's probably best if you explain why an outer join is desirable?

DanChadwick’s picture

My feeling is that the webform views integration is only intended to be executed on webform nodes, and if submissions are displayed, only on the same webform as was used to define the view, a clone of that webform, or a carefully-crafted webform with identical form_keys.

Since an inner join is faster than an outer join and since god knows what bugs there might be if the view were executed on non-webform nodes, my inclination is to leave this as is.

If the poster wants a view that operations on both webform and non-webform nodes, it might be best to write a custom views handler, or to use hook_views_data_alter() as desired.

I think we should mark this works-as-designed.

joshuautley’s picture

@jweowu

An example use case:

I'd like to display both a node and a webform title but I only want to display Open webforms -or- I'd at least like to display the status of the webform as Open or Closed.

Make sense?

And to be more specific here is why I need a LEFT join instead of INNER

I have Webforms and EntityForms. I need to display a list of titles for both and I need to sort on that list.

joshuautley’s picture

@DanChadwick

To be clear I'm proposing a "LEFT" join not an OUTER join.

therealwebguy’s picture

This can quickly be solved by making the Status field provided by webform a LEFT JOIN (you don't want to INNER JOIN this since it won't support any inclusion of other node types)

  /**
   * Webform table definitions.
   */
  $data['webform']['table']['group'] = t('Webform');
  $data['webform']['table']['base'] = array(
    'field' => 'nid',
    'title' => t('Webform nodes'),
    'help' => t('Nodes which have webforms.'),
  );
  $data['webform']['table']['join'] = array(
    'node' => array(
      'left_field' => 'nid',
      'field' => 'nid',
      'type' => 'INNER',
    ),
  );

becomes:

  /**
   * Webform table definitions.
   */
  $data['webform']['table']['group'] = t('Webform');
  $data['webform']['table']['base'] = array(
    'field' => 'nid',
    'title' => t('Webform nodes'),
    'help' => t('Nodes which have webforms.'),
  );
  $data['webform']['table']['join'] = array(
    'node' => array(
      'left_field' => 'nid',
      'field' => 'nid',
      'type' => 'INNER',
    ),
    'status' => array(
      'left_field' => 'status',
      'field' => 'status',
      'type' => 'LEFT',
    )
  );
joshuautley’s picture

Patch for #21 - Please review.

Status: Needs review » Needs work

The last submitted patch, 22: webform_status_field_1110444_21.patch, failed testing.

joshuautley’s picture

Trying this again

visabhishek’s picture

Version: 7.x-3.x-dev » 7.x-4.x-dev
Status: Needs work » Needs review

i am changing status to Needs review with Latest dev branch.

Status: Needs review » Needs work

The last submitted patch, 24: webform_status_field_1110444_21.patch, failed testing.

visabhishek’s picture

Status: Needs work » Needs review
FileSize
399 bytes

Re-rolled patch for #24

DanChadwick’s picture

Version: 7.x-4.x-dev » 7.x-3.x-dev
Status: Needs review » Closed (fixed)

Using the webform views integration on non-webform nodes is outside the use case for webform core. Fortunately, Drupal/Views provides a hook for exactly this situation -- hook_views_data_data(). This will easily allow you to adjust the join and/or add a status field without accessing it via the webform table's view definition -- whichever solution you prefer.

Putting status and version back to the 3-year old original issue. This discussion should have taken place in a new issue anyhow.

therealwebguy’s picture

@DanChadwick - Happy to take this discussion to another thread, but I'd like to better understand your position that this is a non-webform issue. Webform provides views support for its Status field, but does not correctly define its join, forcing its inclusion to INNER JOIN every time. This prevents any conditional querying on anything outside of webform. Thats simply poor design.

If webform is going to provide views integration for a field it defines, it should then be responsible for accurate query definitions. I say all of this with the utmost respect and am in no way interested in starting a debate, I just struggle a bit with the notion that the solution is to leverage hooks programmatically when this is already supported by views.

jweowu’s picture

To be clear I'm proposing a "LEFT" join not an OUTER join.

That's an OUTER join. A "LEFT" join is shorthand for "LEFT OUTER JOIN", just like a "RIGHT" join is a "RIGHT OUTER JOIN".

joshuautley’s picture

@ #30

Ah. Thank you for the clarification on the shorthand.