Comments

danchadwick’s picture

Project: Webform » Webform view
Version: 7.x-4.0-alpha6 » 7.x-1.x-dev
Issue summary: View changes

Moving to the webform view issue queue. Please test with the latest webform 7.x-4.x as there have been view changes made.

miltonmethod’s picture

Im using Webform 3 with Webform Views. I cannot get the webform placeholder to show up in the views formatter. Any ideas?

dman’s picture

Status: Active » Postponed

Structural changes introduced in webform4 meant that large amounts of the processing inside this module would have to be rewritten, if it's even possible. #1981518: webform 4?
A Webform4 compatible rewrite has not been funded yet.

jcnventura’s picture

Status: Postponed » Needs work
Related issues: +#1981518: webform 4?
StatusFileSize
new4.96 KB

I'm submitting a work-in-progress patch that is able to restore compatibility with webforms 4.x.

It's not pretty, as there's a lot of workarounds to the way that webform works. Orders with 'quantity' 0 are not currently being filtered, but it should not be too complicated to use that configuration. I'll probably do it soon.

Note that this patch requires a slight patch to the webform components API: #2537616: Add submision ID parameter to _webform_display_component()

jcnventura’s picture

dman’s picture

Wow, thanks for this effort - I'd personally put it in the 'too hard' basket - at least until I could get another gig that would afford to let me spend 2 days on it.

I feel it's probably worth opening up a new dev branch just for this and seeing if we can go forwards with this!

jcnventura’s picture

Title: Compatability with webform view » Compatibility with webform 4.x

Yes, it would make sense to open a 7.x-2.x branch. There's also a few code style cleanups I could do.. Feel free to add me as co-maintainer, and I'll try to do it :)

dman’s picture

Version: 7.x-1.x-dev » 7.x-4.x-dev
Component: Miscellaneous » Code
Category: Bug report » Task
Status: Needs work » Needs review

I'm pushing the number right up to match the webform numbering system,

The patch was built from webroot, not project root, so could not apply clean. Had to merge in your changes by hand.

I'm using some tools like phpstorm and phpcs now, so some changes to move some bits to this months code standards (like requiring a period at the end of comments ;) can be done as a no-brainer soon.

But first I need to see if this patch can even produce results on current code, so I'm putting this up to run some tests against now...

  • dman committed 90e4e9d on 7.x-4.x authored by jcnventura
    Issue #1957336 by jcnventura: Compatability with webform view (Manually...
dman’s picture

Status: Needs review » Needs work

I tried to get this running last night.
- followed my previous setup instructions directly and got most of the way there.
After eliminating a few warnings, I'm seeing some data get saved in submissions - but getting fatal errors when trying to view the submissions or results.
Inspection and tracing shows that there seems to be a chance to get the right data out again, but needs work still.

I looked at the patch you've got over in webform.module. I'm pretty sure that that will never get in like that, as it consists almost entirely of changing function signatures! HOWEVER, I also found that without the patch, i was getting the whole $submission object turning up as the last argument anyway - in the place where you were trying to insert the $sid. So it may be that the data you were looking for *is* already available.

Lets continue to play in the 7.x-4.x dev branch and see what can be done though.

jcnventura’s picture

Thanks for the heads up. I'm doing this for a client that uses a slightly older version of the webform module (4.9). I'll update it in their site and change the code accordingly. Nice to know that this module doesn't require any changes on the other one.

jcnventura’s picture

Status: Needs work » Needs review
StatusFileSize
new4.11 KB

This code fixes the compatibility with webform 4.10, and restores the required_key functionality which was missing from the previous code (changes to webform_view.module).

Note that at display time I check if nid is present in the submission components. This is because I've only filled the component data structure enough to survive the webform flatten function. If you ever decide (or need) to add the nid field, it will break this check.

jcnventura’s picture

Also, I think that we can remove the _webform_submit_view() function which never gets called. And possibly webform_view_webform_submission_load().

dman’s picture

Status: Needs review » Needs work

I've spent some more time with this, and now I see what is being done there, it really worries me.

The big problem is here:

              // This data structure enables the proper storage of the data in
              // the webform_submitted_data table.
              $form['#node']->webform['components'][$nid] = array(
                'cid' => $nid,
                'pid' => $component['cid'],
                'form_key' => $nid,
                'value' => serialize($val),

- You are interchanging $nid (node ID) with $cid (component ID). These are different things.

When I first saw that I thought it was just a naming mixup, but the results of my tests showed you really are using the NODE ID !!

On my clean test site, I have only 3 resource nodes that I expose in my view.
My simple webform has 5 components.
They are numbered $nid 1,2,3 and $cid 1,2,3,4,5 respectively.
... Can you guess what sort of results I get from this code?

Huge problems here.

The current proposal serializes each view row as one blob each. These may or may not get saved, seemingly getting pushed into the storage space allocated for the 'view' component.
But what really needs to happen is that the parent 'view' component should gather the data from each of its children, serialize that and store it itself in one place.
That's what _webform_submit_view() used to do - though I'm not sure why it's no longer being triggered.
Thenit has to unpack it later, of course, but essentially, all it exposes to the rest of the webform processes is that it's a text blob that looks after its own data. It does not let other webform processes even see its children.

The webform results that get put into the database cannot be saving data using nids instead of cids as keys, and trying to filter them out again by checking

        foreach ($inputs[$form_key] as $nid => $values) {
          if (is_numeric($nid)) {
            // Iterate each node in the view.
            $node = node_load($nid);

^ The $inputs there are Webform Components - and you are casting them into Nodes! (And doing a node_load(), when A: the expected data should already have been submitted and available, and B: we should anticipate that other view-able entities like user or file may be being used.

I don't know how you've got what seems to be working for you to even save and be reflected again to be reported on in results, but it is not working for me. Probably (initially) because I am testing on a brand-new site where nid/cid collisions are happening (fuck did that give me some random results) .
I think this direction is not going well.

First place to look is probably _webform_submit_component() - it seems that's not being triggered, and that's where the magic should be happening - not in form_alter

jcnventura’s picture

Status: Needs work » Needs review
StatusFileSize
new10.78 KB

Well, in my defence the reason why I did such an hack was again that I based all of this on webform 4.9.. The reason was that I needed structured data to survive the flatten, but I needed nice clean textual data to be stored in the normal fields so that the CSV export was correct.

In the meantime, I realize that they've added some calls to set the csv_data and csv_headers. I tried to used them, and again I would have needed the submission info, but even with that I wouldn't be able to make it come out properly. Then I realized that there's a couple of drupal_alter for webform_csv_headers and webform_csv_data that would do it.

This revised patch gets rid of the ugly hack, and stores the structured data in the first defined hidden field under the webfrom_view group. The data is serialized to survive the flatten in is stored like that in the database entry for that field.

It works 100% hack-free(*) and looks good on:
- form submit
- form preview
- result display (individual)
- CSV download

It doesn't yet look too good on:
- results display (table)

I think I'd need again to add the submission info to the API for _webform_table_component(). But then, I realized that it's not that important as the client didn't request a pretty table display.

Please test :)

jcnventura’s picture

Oh, and replying to #14. webform will _never_ call submit for a group. The data can't be saved anymore as part of the group, that's why I resorted to the use of the first hidden field. A further refinement might be to somehow configure which is this 'placeholder' field via the UI, in case some crazy user ever needs to define multiple hidden fields as children of the webform_view group.

dman’s picture

Sounds good.
My initial basic test was -
* submit data
* looks at the results for that submission - in the edit form.

If we can round-trip it enough to save, load, and save that data again into appropriate subforms, then that's a really healthy sign that the data is serialized correctly.
As it didn't produce results in the first attempts I tried, we needed to look at that first.

All the other outputs - email tokens, csv inline rendering, table html view - are all plugin hooks that come after that.
I don't mind seeing the serialized or a json in the initial table renderer for now, but flattening it to almost anything readable that will go into the cell is also possible IIRC.

I can appreciate that you'd have gone straight for saving the input directly as the kind of output you wanted to see today, but we've got to speak webforms language for how it passes data around. This is also necessary so what we do can be (at least mostly) compatible with any other modules that mess with or extend webform.

If there is a problem with webform ignoring 'group', then we may need to find the extra steps that expose it as a *component* that happens to behave like a group - rather that a *group* that has component behaviours. It gets tricky.

I'd like to try this latest variation. I'm a bit sick and not focusing very well this week, so it may take some concentration..
Thanks for the re-work! I know it's quite a lot of changes from what you started off with, but you can see why we needed to do it the webform way :-)

jcnventura’s picture

StatusFileSize
new12.65 KB

Newer version of the patch, this time including a new feature. If the embedded view is marked as required (in the webform field setup page), then the form submission will require validation that at least one value in the required field has been submitted. This prevents empty sets of 'orders'.

  • dman committed 06660cc on 7.x-4.x authored by jcnventura
    Issue #1957336 - Compatibility with webform 4.x - patch 18
    (cherry...
dman’s picture

Status: Needs review » Needs work

THANKS!
I'm having a LOT more joy with this now. I am successfully seeing data going in AND being displayed again, without troubles!

However, each time I've been patching, I have also had to repair minor PHP strict warnings. In this case I pay attention to these issues, as it's likely that they actually mean something is missing deep in the internals. I've been putting those into the git history on the 7.x-4.x branch, but your patches haven't been pulling those, and come from various points different in the git history. The commit hashes I see in the patches don't line up with the webform_view project, so I'm guessing they are coming from your own project?
I've been trying to use drush-iq for patch management here, but it's getting confused each time.

Anyway - So I have
* Applied your patch from #18, over-writing the now-garbled history in the 7.x-4.x branch where I'd already added two of your previous variations. It's in the repo as a living -dev branch
* Abandoned my attempts to repair the PHP warnings for now

> Notice: Undefined index: submitted in webform_view_form_webform_client_form_alter() (line 103

I'm seeing the data come out again nicely enough in both the submission view table and the CSV export (YAY!)
The results table is still serialized (OK for now, can be fixed later)

But I'm not getting it unpacked and put back into the node edit form - and that's needed.

* I will have a short crack at that now.

Please treat our 7.x-4.x branch as an unstable development sandbox until we get this right. We can continue to break things until things are fixed.

In fact, I'll add you as a maintainer now, as you suggested above, now that I'm happy that we are going down the right path and we are seeing progress!
Please pull and push as needed directly on this dedicated feature dev branch during this phase!

jcnventura’s picture

Status: Needs work » Fixed

Basic compatibility is working. We should continue to fine tune via e-mail. No need to be so public about the next (minor) sub-steps.

Status: Fixed » Closed (fixed)

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

pepemty’s picture

Sorry to reopen this issue, but I have to ask:
Will there be a fix to the Webform 4 incompatiblity?
Will there be a Drupal 8 version?

Thanks for all the hard work.

Warm regards from sunny México!

: )

jcnventura’s picture

@PepeMty: there is a fix to Webform 4 in the 7.x-4.x branch.

There will probably not be a Drupal 8 branch until webform itself is ported to D8 AND it becomes evident that it's needed.. At the moment, it seems that a lot that could only be done with webform can now be done with the core contact module.

matia’s picture

Hi All,
can I have a copy of patched module after applying patch 18, I tried to apply it but it applied only partially with net beans?

update: sorry I was applying to wrong version of the mosule I will try now with 7.x-4.x