Provide form support

@adding handlers:

have a look at the pageroute drupal 6 branch, pageroute.module, function "pageroute_after_build".

redirect target -> look at page redirect action for the config form.

>There is a setting to display the form ID for Rules admins when they visit a page with a form (to ease the lookup of which form ID needs to be enabled)

Hm, I think we could even restrict that to the current session. So other admins aren't confused by those messages.. ;)

>Problem: How can I get the complete list of form IDs that are available on a site?

I think we won't need that any more. The message knows about the form id and can pass it in the link to enable the event for that form.

> because triggering events on really every form on a site could make it slow

Just putting my thoughts out there - I wonder what is the time loss. On most pages I see there are 2-3 forms (e.g. node_form, search_form, devel_form). Maybe triggering on every form is a valid solution, that will remove the complication of selecting the form ID in advance?

>but this doesn't work, since not all necessary files are included and the function is not found. Any hints?
Hm, I think you can't as there is no way to know the right include file. Yep, that's odd.

I can just think of a hack to achieve that:
* Just link to the current page with a flag set like "add_event=1"
* When back at the form read out that flag + check the id to match form_alter. If it matches add the event and redirect to the rules forms admin page.

However we could also live without checking the id. I leave that up to you. :)

I did a short review of the patch:

* Watch out for your event argument labels to be translatable. I think you could just define the labels in your helper function as they probably stay for all form events.

* +class rules_data_type_form extends rules_data_type {}
If you don't change the class, just use the rules class.

* I think you have to set "uses input form" to FALSE, as it defaults to "!identifiable" - thus you get the usual "variable select" element (and no direct input form).

@#6: Indeed, we thought about that. As there is probably a form on almost any page (user login block..) I think it's better to let rules only operate when really needed. I think it's fast enough that it wouldn't hurt really, but having more fine-grained event just ensures better site performance thus in future we can even avoid to include the rules engine when not needed. Also else the user would have to check for the form_id manually - but the id is difficult to identify for the user. Thus I think it's even more usable that way.

I just did a visual code-review. It looks already good, I've not tested it yet though.

* Custom Form ID label => Why not just call it "Form label" ? It isn't really the ID you label, but the form I think.

* Element ID to adjust weight

I think a common and unique label suffices in that case, like "Form Element ID". We usually don't append the action to the label as the whole form is in the context of configuring the action nevertheless.

* + if (in_array($form_id, array_keys($form_events))) {

I think it's much faster to just check isset($form_events[$form_id]) as so php doesn't need to search through the array.

* '@elem'
Use whole words for the replacements, as for variables.

@$display_form_id_users:

Hm, why not just rely on $_SESSION for that? Just set a boolean there and check it in hook_form_alter if it's there.

Also I think for the validation use case we need a condition like "Form element has value", so you can check whether the user has specified a given value.

subscribing

+ foreach ($form_events_changed as $form_id => $value) {
+ if (empty($value)) {
+ unset($form_events[$form_id]);
+ }
+ }

You can do just $var = array_filter($var) instead of that.

+ $form_id_msg = $form_values['values']['enable_form_id_message'];
+ if ($form_id_msg) {
+ $_SESSION['rules_forms_message'] = TRUE;
+ }
+ else {
+ $_SESSION['rules_forms_message'] = FALSE;
+ }

$_SESSION['rules_forms_message'] = (bool) $form_values['values']['enable_form_id_message'];
:)

>+ $msg = t('Once the activation is confirmed, events are triggered on this form and can be used in rules.');
Terminology: Events aren't triggered, they occur when they are invoked and trigger rules.
Then you have in .module:
+ // Fire event if form is enabled
+ rules_forms_trigger_event('form_built', $form, $form_state, $form_id);

>+ drupal_set_message(t("%form has been activated.", array('%form' => $form_state['form_id_activate'])));
Perhaps use http://api.drupal.org/api/function/drupal_ucfirst for the id. Also in the event labels.

Coding style:
in rules_forms_events_arguments the arrays should be in one line as long as the lines get not longer than 80chars.

@rules_forms_action_set_error
Does that work if one has deep nested arrays like test[test][test][test] too? Also you should comment what you string-magic there does.

@rules_forms_condition_element_value
Is the type-safe comparison === working for numeric values right? I think it should, just want to be sure.

@redirect action:
Redirection to paths with query and fragment should work too. (see page redirect action)

+ $form['#submit'][] = 'rules_forms_event_submit';
+ $form['#validate'][] = 'rules_forms_event_validate';

That won't work when a submit-button comes with customized callbacks. You have to implement the pageroute-workaround I mentioned for that.

Suggestion:
Provides events, conditions and actions for forms. --> Provides events, conditions and actions for rule-based form customization.

@rules-scheduler:
This change shouldn't be in this patch. I just took it and committed it.

That's it :) Apart from that we need to mention the module in the README and explain there what it is and the basic steps how to use it. Also once done, we should generate a translation template with the potx module. Also I've to do a test-run, I'm going to do so later today.

I just did another visual review, looks fine now. Just one minor point:
* Why is $action_args used when we are invoking an event? Imo a bad variable name.

I'm going to give it a test-run now.

Oh, watch out for notices..

* notice: Undefined index: rules_forms_message in /var/www/fago/web/drupal-6/sites/all/modules/rules/rules_forms/rules_forms.module on line 60.
* notice: Undefined index: rules_forms_message in /var/www/fago/web/drupal-6/sites/all/modules/rules/rules_forms/rules_forms.module on line 60.
notice: Undefined index: form_events in /var/www/fago/web/drupal-6/sites/all/modules/rules/rules_forms/rules_forms.admin.inc on line 42.
notice: Undefined index: #suffix in /var/www/fago/web/drupal-6/sites/all/modules/rules/rules_forms/rules_forms.module on line 110.

Minor issues:

admin/rules/forms
------------------
1) Move the "Overview of forms on the site that are enabled to trigger form events"title under the "Enable form ID event activation message" checkbox.
2) Maybe add a "no form selected yet."

admin/rules/trigger/add
------------------
3) Maybe the event group should be called "Rules forms" instead of "Forms".

* Only visible for this session.

Session is a bit technically, perhaps better say "applies only to the currently logged in user."

Agreed @Forms group.

@value condition:
It would be good to be able to use that for multiple-selects too. I'd suggest using a textarea for the value and allow to enter one value per line.

* Something strange seems to be happening with the form_set_error action. When I use the build form event and a simple form element name like "title" it just display the element title as error message? It's working in another action, but I think that might stem from a cached action info inside the action. Perhaps you just mixed up the argument order there?

Apart from that it looks already very handy!

I wonder though if we should make it possible for users to re-use the form values in some actions without using php. A way would be to use the "add new * variable" actions, but to prevent the need of recreating all these we could do a "input evaluation" filter that takes the element id and replaces it with its value.

Anyway we need to think about that, so if we decide to do it we can tackle that later on.

Great work, thanks!

I just improved the description in the main README a bit and committed it.

I also granted you CVS access, so you can commit any further rules-forms fixes on your own :) Perhaps we should also mention that you maintain rules-forms in the readme?

ah, also the module needs a translation template like the others. Check out the potx module for generating it.

@klausi,
Congrats! and keep on the good work :)

fyi,
I've created first patch using rules forms :) - #538834: Rules action - disable a form element

I wrote a tutorial at http://drupal.org/node/794280

Thanks all contributors for this new feature. It's awesome :)