The description in webform_drush_command() is wrong. The component option keys have to be prefixed with "webform_".

Comments

danchadwick’s picture

Component: Documentation » Code
Status: Needs review » Needs work

Confirmed, but it's not clear what we want. I think the intention of prefixing the submission info fields with webform_ is to prevent conflicts with actual component form keys. But the command doesn't accept form keys, but rather component cid's.

I see three options for this isssue:

  1. Your #1 patch.
  2. Leave description, but strip by add webform_ prefix for the user for non-numeric values.
  3. Leave webform_ prefix as required, but also accept form keys in addition to cids, and adjust description to suit. A integer entry is a cid, an entry that starts with webform_ is a submission info, and anything else is a form key.

I like Option 3 myself. Comments?

morenstrat’s picture

Status: Needs work » Needs review
StatusFileSize
new2.07 KB

I'm also in favor of option 3 and created a patch for it.

danchadwick’s picture

Status: Needs review » Needs work

Very nice. My comments are trivial and stylistic only. I haven't tested it yet, but it looks good.

  1. +++ b/webform.drush.inc
    @@ -65,6 +65,20 @@ function drush_webform_export($nid = FALSE) {
    +
    ...
    +
    

    Unneeded blank lines.

  2. +++ b/webform.drush.inc
    @@ -65,6 +65,20 @@ function drush_webform_export($nid = FALSE) {
    +    $form_key = $component['form_key'];
    +    $form_keys[$form_key] = $cid;
    

    Combine these lines, I think.

  3. +++ b/webform.drush.inc
    @@ -65,6 +65,20 @@ function drush_webform_export($nid = FALSE) {
    +
    

    Need an unset of $component. Reference loop variables are PHP timebombs.

morenstrat’s picture

Title: Wrong keys in drush component option description » Fix drush component option description and allow for using form keys in component option
Status: Needs work » Needs review
StatusFileSize
new2.06 KB

New patch attached!

danchadwick’s picture

Status: Needs review » Fixed
StatusFileSize
new2.49 KB

Thanks very much!

To clarify my third comment, I don't object to reference loop variables, but they need to be unset immediately after to loop. If you happen to (maybe years later) reuse that variable name, it will still be a reference, leading to much hair pulling. I restored your original version of the loop and added an unset.

Committed to 7.x-4.x and 8.x.

Also, one tiny request: if you name patches according to the Drupal convention, it is easier for maintainers to track them.

  • DanChadwick committed 92aa2e0 on 8.x-4.x authored by dunix
    Issue #2532878 by dunix: Fix drush component option description and...

  • DanChadwick committed 7a04143 on authored by dunix
    Issue #2532878 by dunix: Fix drush component option description and...

Status: Fixed » Closed (fixed)

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