Following up on #1215456: Multiple and same-page conditional rules although there is nothing directly related in the thread.

A suggestion to improve the usability is to group targets into a multiple select list, so if 4 or 5 targets all components related to the same condition, so these can be selected in bulk. In my case I have around 30 elements off a single question - "If no skip the next 5 pages", so there are around 30 conditional actions all based off this one condition.

This could be implemented completely on the form itself, grouping identical conditionals together and changing the target to #multiple and then pulling these into individual components again on submit. Or without looking at the code / schema, to make the condition reaction relationship 1 - n, but I guess that would be a lot more work, although it would make things more extendable latter (ie. Group different reactions off one conditional: hide these ones, make these ones required, etc).

Other than this, this feature appears to be working very nicely, so a big big thanks on everyone involved!

Files: 
CommentFileSizeAuthor
#31 webform-default_argument-1815996-31.patch1.28 KBDanChadwick
#25 webform-multiple_targets-1815996-25.patch31.2 KBDanChadwick
#16 webform-multiple_targets-1815996-16.patch31.24 KBDanChadwick
PASSED: [[SimpleTest]]: [MySQL] 1,782 pass(es).
[ View ]
#15 webform-multiple_targets-1815996-15.patch30.4 KBDanChadwick
PASSED: [[SimpleTest]]: [MySQL] 1,782 pass(es).
[ View ]
#12 webform-multiple_targets-1815996-12.patch29.36 KBDanChadwick
FAILED: [[SimpleTest]]: [MySQL] 1,656 pass(es), 126 fail(s), and 3,654 exception(s).
[ View ]

Comments

Alan D.’s picture

Sweet, no more need for select or other either! This is awesome!

quicksketch’s picture

"If no skip the next 5 pages", so there are around 30 conditional actions all based off this one condition.

This doesn't entirely solve the original request, but it could make life easier for you: You could select the page breaks to be the target of the conditional instead of the fields, so you would only need 5 conditions instead of 30.

Alan D.’s picture

Thanks, this works a treat! Maybe the page break hint could be added as some help text in the admin UI

- Conditionals may be used to hide or show certain components (or entire pages!) based on the value of other components.

+ Conditionals may be used to hide or show certain components (or entire pages by targeting page breaks) based on the value of other components.

or

+ Conditionals may be used to hide or show certain components / pages based on the value of other components. Hide pages by targeting the page break of the page you want to hide.

Leaving open in case someone wants to investigate more with original issue, but happy if this is closed as a won't fix / works as designed.

Simon Georges’s picture

MilOrg’s picture

StatusFileSize
new31.56 KB
new24.72 KB
new15.57 KB
new23.68 KB
new14.93 KB

A little followup to Alan's post (which had some great feature requests).

The way the conditionals work now, you can add a second requirement for the action to take place:

Clicking the "+" gives you:

Would it be possible to add a second set of "-/+"-buttons to a conditional to also add an extra action?

Perhaps something like:

that would either result in

or

?

cfaaadmin’s picture

Yes, yes, yes! This is exactly what we need for Webform...

quicksketch’s picture

Title:Conditional UI enhancement» Multiple target components for each conditional
Priority:Minor» Normal

Hi guys, yes of course "it's possible" but it's a matter of implementation difficulty. Right now we have a many-to-one relationship between conditionals and actions, and this would require a many-to-many relationship, meaning we'd need to add a 3rd table: webform_conditional, webform_conditional_rules, and (the new table) webform_conditional_targets.

I'm happy to look at patches for this feature, but I probably won't give it a priority for a while.

Thanks @MilOrg for the excellent screenshot demonstrating the desired functionality.

strategery’s picture

I really need this functionality. Right now I am trying to code an inspection form, so that the user can specify the number of individual locations that need inspection. Each location has the same things that need checked, but the number of locations can range from say 1-60. I need to be able to specify the number of fieldsets based on the number of locations indicated by the user. The only way I think that can work now is if I could add multiple conditions based on a single trigger, rather than the other way around. Or I could create 60 different pages, but that's cumbersome. Unless there's another way, I would love this to work.

maritimefist’s picture

Is there any news on this? I too could greatly benefit from this feature.

DanChadwick’s picture

Issue summary:View changes

This is a good idea that awaits a patch. I'm not sure that I agree with quicksketch that another table is needed. We already have code that processes the conditionals to determine which fields need to be sent to the browser. The soft rule would still be that each component is at most a target of one conditional (otherwise a warning is issued). It's just that a conditional could have multiple targets, all of which can't appear in another conditional.

BTW, if the targets are grouped in a fieldset, hiding the fieldset hides all its children. This is an implicit variation on this feature.

Also note that this feature may interact with the feature to use conditionals to set values.

DanChadwick’s picture

This is a good idea and more implementable now that conditionals work in dependency order. Despite what I said in #10, we would need to add another table for the actions and a hook_update_N to migrate the single target from the rule group to the actions table. Code for deleting components would need to delete them from actions too, including deleting actions that no longer have any targets. And of course the code to analyze dependencies, execute conditionals in the server, and execute conditionals in the browser will all need adjusting.

The design in #5 is the right way to go. This sets the stage for actions other than show/hide.

Somewhat related, I'd like the show/hide to be changed to when the condition is true/false SHOW the component. This sets the stage for changing components with conditionals.

DanChadwick’s picture

Priority:Normal» Major
Status:Active» Needs review
StatusFileSize
new29.36 KB
FAILED: [[SimpleTest]]: [MySQL] 1,656 pass(es), 126 fail(s), and 3,654 exception(s).
[ View ]

Another big patch.

webform.components.inc.

  1. The code which handles deleting portions or all of a conditional when the component is deleted was updated for when the component is used as one of possibly many actions. In addition a subtle bug where a component may be used more than once as a source was fixed. (E.g. if a=1 or a=2 then hide b).

webform.conditional.inc.

  1. Conditionals with the last action removed are removed from the form, matching the code for removing the last rule.
  2. A new rule has an array of actions, rather than one target/action
  3. Form validation for defining conditions was updated for any of its targets' components being any of its rules' components. In addition, you can't use a target in more than one action of a conditional.
  4. Saving conditionals create an array of actions, rather than just one.
  5. The conditional for was adjusted to allow adding/removing multiple actions.
  6. The code which responds to an ajax + or - was fixed. There was a bug causing a NULL reference when removing the last conditional. Users would typically not see the PHP notice because ajax operations don't display them.
  7. The code for expanding a condition and theming it was modified to cope with multiple actions.
  8. Actions are written to a separate table.
  9. A deleted conditional deletes its actions from a separate table.
  10. The javascript setting sent to the browser for the client form includes an array of actions.

webform.webformconditionals.inc

  1. The rules were enhanced to allow for an array of actions (i.e. targets), rather than just one.
  2. The code to execute the conditionals on submission data was enhanced to allow an array of actions.

webform-admin.js

  1. The code to remove the entire row from the table was enhanced to also do this when the last action is removed. Due to running jquery 1.4, this was harder than expected due to the lack of $.proxy(function, context, extraarg).

webform.js

  1. The code to execute conditionals was enhanced for an array of actions.

webform.install

  1. A new table was added to the schema to store actions (webform_condtional_actions). I picked a plural name to match webform_conditional_rules, although I prefer table names to be singular. This table contains two extra columns for future use: invert (to provide an "else" option) and argument (to provide an argument for setting actions, such as set a = 1.
  2. A hook_update_N was added which creates this table, copies the data from webform_conditional into it, drops the unneeded columns from webform_conditional, and rebuilds the class registry. This last step will help site upgrading to the main conditional function, where core needs to discover WebformConditionals.

webform.module

  1. The webform_conditional_actions table is removed upon uninstall.
  2. The actions are loaded from the new table upon loading the webform node.
  3. The code for performing a rule check on adding components to the client form was enhanced to multiple actions.

Hopefully this patch will get a review. This patch is needed to continue work on other conditional issues.

Status:Needs review» Needs work

The last submitted patch, 12: webform-multiple_targets-1815996-12.patch, failed testing.

DanChadwick’s picture

Status:Needs work» Needs review

Setting back to Needs Review, but clearly the tests need updating. (I'm hoping this won't re-trigger the testbot.)

DanChadwick’s picture

StatusFileSize
new30.4 KB
PASSED: [[SimpleTest]]: [MySQL] 1,782 pass(es).
[ View ]

Let's try this again. One part fixed tests to adapt to multiple targets, one part fix webform to correctly evaluate multiple targets in server. This check rule code really needs to get simplified using the data for the topological sort, but this patch has to go in first.

The conditional test pass on my development server now. First time since my involvement in webform that they've been helpful. I'm not sure what to think of that.

DanChadwick’s picture

StatusFileSize
new31.24 KB
PASSED: [[SimpleTest]]: [MySQL] 1,782 pass(es).
[ View ]

Revised only in that the update message suggests a backup before proceeding.

I have committed this locally to allow development to continue on related issues. I won't push it until it's had a little more time to see if it attracts any comments.

mitch147’s picture

I have applied the patch however when i go to the conditional tab of the webform I receive a Fatal error: Class 'WebformConditionals' not found in on line 3824.

DanChadwick’s picture

Re #17 -- Just to confirm -- you are applying the patch for testing purposes only, not for use in production. This patch or the implementation may well change or even never be committed (unlikely).

This patch should be applied to the latest dev, which is 7.x-4.6+6-dev

If you are getting that Fatal error, either you haven't cleared your cache or you applied the patch to an older version of webform.

This patch requires that you run update.php or drush updatedb. The update makes non-reversible changes to the database schema. Be sure you have a backup you can restore.

mitch147’s picture

I am obviously doing something very wrong and I imagine it is something very simple I have missed. The patch file is located in the most current directory of webform. I use git apply patchname.patch and nothing occurs? I receive the following error.

patch.name:719: trailing whitespace.

warning: 1 line adds whitespace errors.

This is being patched locally in development, on receipt of the error no files have been changed. I have spent hours and I thought it had worked then had issues with the patch working.

DanChadwick’s picture

@mitch147,

  1. Ensure that git is installed properly and working.
  2. Clone the repository into a the module directory of your choice (e.g. sites/all/modules) following the directions from the module page: https://www.drupal.org/node/7404/git-instructions/7.x-4.x/nonmaintainer
  3. Apply the patch following these instructions: https://www.drupal.org/node/1399218

This patch can currently be applied to the head of the 7.x-4.x branch.

mitch147’s picture

I have solved the patching issue, it was associated with being on the wrong branch. I have attempted to apply a conditional with 3 targets and resulted in the following error:
PDOException: SQLSTATE[42S02]: Base table or view not found: 1146 Table 'infit.webform_conditional_actions' doesn't exist: INSERT INTO {webform_conditional_actions} (nid, rgid, aid, target_type, target, action) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5); Array ( [:db_insert_placeholder_0] => 1 [:db_insert_placeholder_1] => 0 [:db_insert_placeholder_2] => 0 [:db_insert_placeholder_3] => component [:db_insert_placeholder_4] => 3 [:db_insert_placeholder_5] => hide ) in drupal_write_record() (line 7239 of C:\Users\mitch\Sites\devdesktop\infit\includes\common.inc).

What has caused this exception?

DanChadwick’s picture

You didn't run update.php.

mitch147’s picture

Thankyou, the patch is now functioning correctly. From a functionality point of view, I have one more query.

An example:
I have a listbox with three numbers (1,2 and 3). These numbers correspond to showing three fieldsets. I have tried different conditions to achieve this however am still not able to and I originally thought this patch was my solution.

The following are the conditions I am trying to apply:
1. When the listbox is empty none of the three fieldsets are showing.
2. When the number '1' is slected from the listbox the first field set is showing and fieldset '2' and '3' are hidden.
3. When the number '2' is slected from the listbox the first and second field set are showing and fieldset '3' remains hidden.
4. When the number '3' is slected from the listbox the first second and third field are showing with none hidden.

I have tried different logic to attack this however I believe the issue is associated with sequencing the conditons. I am unsure of a solution. My solution (im not but a humble reliability engineer) is perhaps to allow multiple fields to be hidden under one condition and if the condition is not true then that condition is ignored irrespective of the sequencing.

DanChadwick’s picture

Sequencing is now irrelevant. Webform analyzes the dependencies in the list of conditionals and build a correct sequence so that when a conditional is evaluated, all its source components have already been evaluated.

When you hide a fieldset, it hides everything in it.

What you want is

if s =1 or s = 2 or s = 3 then show fieldset a
if s = 2 or s = 3 then show fieldset b
if s = 3 then show fieldset c

Three different conditional groups, unrelated to this issue.

DanChadwick’s picture

Status:Needs review» Fixed
StatusFileSize
new31.2 KB

Resolved upstream conflicts and committed to 7.x-4.x.

DanChadwick’s picture

Category:Feature request» Task
Priority:Major» Normal
Status:Fixed» Patch (to be ported)

Needs port to D8. fenstrat -- For the 4 pending conditional patches, apply this one 1st.

  • DanChadwick committed 502b4f1 on 7.x-4.x
    Issue #1815996 by DanChadwick: Multiple target components for each...
DanChadwick’s picture

Version:7.x-4.x-dev» 8.x-4.x-dev
fenstrat’s picture

Version:8.x-4.x-dev» 7.x-4.x-dev
Category:Task» Feature request
Priority:Normal» Major
Status:Patch (to be ported)» Fixed

Committed and pushed to 8.x-4.x. Thanks!

  • fenstrat committed ed5d728 on 8.x-4.x authored by DanChadwick
    Issue #1815996 by DanChadwick: Multiple target components for each...
DanChadwick’s picture

StatusFileSize
new1.28 KB

Small problem with the hook_update. The argument wasn't being set to an empty string, and text fields can't have defaults.

Committed to 7.x-4.x. No D8 port needed because it is an hook_update function.

  • DanChadwick committed 1df2b1c on 7.x-4.x
    Issue #1815996 by DanChadwick: Set default empty argument for...

Status:Fixed» Closed (fixed)

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