Problem
After update to webform 7.x-4.14 I found a php Notice was being always displayed after submit a webform.
Digging in the code I found the problem was in the way the function webform_submission_render is excluding components.

// Remove excluded components.
  if (is_array($excluded_components)) {
    foreach ($excluded_components as $cid) {
      unset($components[$cid]);
    }
    if (!empty($email['exclude_empty'])) {
      foreach ($submission->data as $cid => $data) {
        // Caution. Grids store their data in an array index by question key.
        if (implode($data) == '') {
          unset($components[$cid]);
        }
      }
    }
  }

When $data is a matrix, the implode will return an error (we only can implode strings not arrays).

Solution
Take into account the $data variable could be a multidimensional array. The previous module version code will work fine.

 // Remove excluded components.
  if (is_array($excluded_components)) {
    foreach ($excluded_components as $cid) {
      unset($components[$cid]);
    }
    if (!empty($email['exclude_empty'])) {
      foreach ($submission->data as $cid => $data) {
        // Caution. Grids store their data in an array index by question key.
        foreach ($data as $value) {
          if ($value != '') {
            // This component has a non-empty value. Continue the outer loop.
            continue 2;
          }
        }
        unset($components[$cid]);
      }
    }
  }

I know it's not the most efficient approach but I think we should roll back to the previous version to avoid problem and then think in a better way to do it.

There is another issue that maybe related but it was closed recently , #2601546: Improve "exclude_empty" processing.. The approach in that ticket is use implode, so the problem will still be there.

Comments

fran seva created an issue. See original summary.

fran seva’s picture

I created a patch that revert the code to the previous version and fix the problem.

fran seva’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 2: webform_submission.exclude_empty-2832175-1.patch, failed testing.

liam morland’s picture

Do I understand correctly that you see this every time a form is submitted if it contains a grid component?

fran seva’s picture

StatusFileSize
new794 bytes

I have recreated the previous patch.

fran seva’s picture

StatusFileSize
new106.91 KB

Hi @liam-morland,
The way I have to reproduce the error is use addressfield module but the error will happen everytime the implode receive a grid.

selection_132.png

I'm thinking how create a test to reproduce the error without use addressfield. I'm not sure how to mock this use case.

fran seva’s picture

Status: Needs work » Needs review
liam morland’s picture

Issue summary: View changes
liam morland’s picture

Your patch seems to reverse #2601546: Improve "exclude_empty" processing.. We need a solution that doesn't cause that problem to come back. Maybe _webform_client_form_submit_flatten() can be used.

liam morland’s picture

Status: Needs review » Needs work
fran seva’s picture

Assigned: Unassigned » fran seva
fran seva’s picture

Assigned: fran seva » Unassigned
Status: Needs work » Needs review
StatusFileSize
new817 bytes
new817 bytes

Hi -- I've been working on it and I'm not sure if we can use _webform_client_form_submit_flatten() or _webform_components_tree_flatten.
Looking in Drupal core, I found we could use options_array_flatten function that flat multidimensional arrays without take into account any special key component as previously functions do.

What do you think about it? Do you think _webform_client_form_submit_flatten should work?

Status: Needs review » Needs work

The last submitted patch, 13: webform_submission.exclude_empty-2832175-13.patch, failed testing.

fran seva’s picture

Status: Needs work » Needs review
StatusFileSize
new621 bytes

Sorry, I created the patch using the wrong branch.

fran seva’s picture

liam morland’s picture

Thanks for the patch. Can you write a test for this?

fran seva’s picture

Assigned: Unassigned » fran seva
Status: Needs review » Needs work
Issue tags: +Needs tests
igorski’s picture

Version: 7.x-4.14 » 7.x-4.18

Liam, maybe I can write a test. Can you elaborate on what’s needed?

liam morland’s picture

Version: 7.x-4.18 » 7.x-4.x-dev

It would be great to have a test that would submit a test form containing the sort of component that would cause this problem. The test would fail without this patch.

liam morland’s picture

Status: Needs work » Closed (outdated)

Drupal 7 is no longer supported. If this applies to a supported version, please re-open.

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.