Action to produce data

Yep. In 6.x there is a "add variable" action. I think we need something like that for 7.x too. First off make the data type configurable, then as next step we provide the input form for the data type - then in case if it is 'struct' or a sub-type of it we can turn each property into a parameter, for which the user may provide a value. For nested complex types, the user just would have to create the need complex type beforehand...

I don't think sebgil plans or has time to work on this though.

Great! So far so good. Does natcasesort() keep the keys? RulesPluginUI::getOptions() so far uses asort().

So far so good. If the data type has a separate input form, we should use that. Else if it has property info + a creation callback we can use that. Else, we cannot create a new data item of the type. In this case we also need to remove it from the options.

I'd suggest to turn on the 'named parameter' feature - that way dealing with the passed arguments is much easier. You just get the keyed array $arguments and $element.

>> If the data type has a separate input form, we should use that.

Yes, I'm copying over the data info currently.

If it has an *input form*. There might be data structures coming with property info that implement their own input form. Indeed we have that already in rules with 'text_formatted'.

>Why? If we have properties we can assume it is a simple array structure and create that. If we don't have anything we can just provide a field to allow single values.

Still there is the possibility of a data type coming without any property info at all. Defaulting to simple arrays would be possible, however this makes the callback a mandatory. I don't think this is a good idea, as it might be un-needed or even un-possible in some cases.
Just using "array" for data structures not fitting into that (object trees, differently organized arrays) is not possible as that way we would create a wrong $data instance and lead to errors in further use (e.g. when used by a certain action for that type).

Sorry, I don't understand. So the property info would contain a key 'input form' => 'my_custom_form_callback'?

No. Rules data types can have an UI class implementing the interface for direct input - as text_formatted have. In that case adding a single input parameter with the given type does it.

+++ rules/modules/data.eval.inc
@@ -111,6 +111,26 @@ function rules_action_data_list_remove($list, $item) {
+  if (isset($type_info['property info'])) {
+    $data = $values;
+  }
+  else {
+    $data = $values['value'];
+  }

If you don't name the value parameter 'param_value' but just value, you can just check for the existence of that paramater and don't to fetch the cache again.

+++ rules/modules/data.rules.inc
@@ -532,6 +558,59 @@ function rules_action_data_list_add_positions() {
+      if (isset($cache['data_info'][$property_info['type']])) {
+        $element_info['parameter']['param_' . $property] += $cache['data_info'][$property_info['type']];
+      }

What are these lines for?

Also you might want to clean the property_info array so only valid properties are copied over. array_intersect_key is fine for that as it also ignores not existing keys.

Powered by Dreditor.

Code looks already good :)

Best use RulesPluginUI::getDataTypeClass(). (This should be static) + $class instanceof RulesDataDirectInputFormInterface for checking for DI. That should be shorter. Then make the if() to be a single line even if long, two lined ifs are really weird to read.

For checking for entities you could use:
+ $data_info = $cache['data_info'];
+ $entity_info = entity_get_info();
-> $data_info = array_intersect_key($cache['data_info'], entity_get_info());

Also you don't need the extra check for 'entity', it won't have a creation callback anyway or? :) Entities too, but let's keep that to make sure they can't be double-covered (entity_create + data_create).

+ *   - 'creation callback': Optionally, a callback that creates a new instance of
+ *     this data type. See entity_metadata_create_node() for an example.

Please clarify the relation to the entity creation callback in the comment. Also mention rules_action_data_create_array() there - maybe this is the better example? rules_action_data_create_array() is broken as it doesn't generate a wrapper though.

+    'callbacks' => array(
+      'form_alter' => 'rules_action_entity_type_form_alter',
+      'validate' => 'rules_action_entity_create_validate',
+    ),

Great that we can reuse those functions, however it's not obvious that they are reused. Best find a generic name for the function and specify it for both actions - that makes it more obvious. Also comment the functions to be used by both actions.

+    $data = entity_metadata_wrapper($args['settings']['type'], $args['settings']['value'])->value();
+    return array('data_created' => $data);
+  }

Just return the wrapper, so Rules can re-use the object instead of having to create another one.

Also we need to add a test for it - at least with text_formatted. A test case with property info would be good too, but I can't think of a use case for it in core. But the testing module could add a creation callback for watchdog entries.

Thinking about that I wonder how this relates to the "Add a variable" actions we had in 6.x. For sure we need something like that in 7.x too. Perhaps we should just split the action into two and use "add a variable" for *any type* - what gives us the DI use case too and concentrate on data structures with data_create() ?

>instanceof does not work here, as we deal with class names (strings) and not objects.
oh, indeed. I remembered that somehow this is necessary, bot got distracted by a wrong use of instanceof in the ui. Fixed that :)

>* Now with simpletests.
Awesome!

>* creating more than one variable in a rule
You can just add multiple variables? Ah I think what's the problem right now is missing UI support for renaming the provided variable.

What data_create does different to "Add a variable" is that it creates *new* data. I cannot use the action to create a new user variable from node:author, or a new text variable 'foo'. The "Add a variable" would allow that and can support any data type. If there is no data source available + no direct input mode = bad luck .. ;)

But if we have that action, data_create does not deal with DI-capable data types any more as you can use "add var" for that. What remains is creating new data structures, what isn't properly possible with "add var" as you cannot allow data selection of a single var or the input of *multiple* parameters -> direct input for data structures is not possible such that you can use data selection for filling the single properties.

+/**
+ * Creation callback for array structured data.
+ */
+function rules_action_data_create_array($values = array(), $type_info = array()) {
+ // $values is an array already, so we can just pass it to the wrapper.
+ return entity_metadata_wrapper('struct', $values, $type_info);
+}

Passing through $type_info is wrong here - it is not the same info. Best just use rules_wrap_data() to create the wrapper, as it incorporates property info from the data info hook. Also the wrapper should be created with the data type as registered in rules.

Patch looks good.

+ * - 'creation callback': Optionally, a callback that creates a new instance of
+ * this data type. See rules_action_data_create_array() for an example.

As noted, we should clarify that entities should use hook_entity_info() 'creation callback' instead, as introduced by entity metadata.

>Are you planning to implement a generic ui interface to rename any provided variable of any action?

Yes. #869398: UI for renaming/labelling provided variables

>You mean you want to clone node:author? If you want to create a new user, you should use the "create entity" action. Or do you want to have just a virtual user object that isn't saved automatically?

I create a new var $my_user and put node:author in there - for whatever reason. Maybe I change node:author later on. $my_user internally just points to the entity as node:author does.

>So what am I missing?

The action provides no way to put existing data into a new variable as it restricts the value to input. I just had a look at add_var in 6.x, and it behaves the same. But in 6.x we had no reason to support adding a new variable with the value of an existing one, as there were no data selectors like node:author - thus in 6.x we would have just duplicated the variables.

But for 7.x there is the use-case of changing a data property and keeping the original value in a variable. Are there other use cases? I don't know. Maybe
* shorten a long data selector node:referenced-node:author:node-profile-type:comment to comment? :D
* providing vars in components. To provide node:author as variable, you need to have a variable for it.

Supporting add_var too is as easy as removing the restriction to input and fixing the type options accordingly. However the problem is that the action name data_create doesn't fit any more :( Additionally one needs to be able to create a var for data-structure types by copying an existing value too. Thus we would have to split the action into two => variable_add + data_create (only for data structures).

That's fast.. :)

+++ rules/modules/data.eval.inc
@@ -111,6 +111,44 @@ function rules_action_data_list_remove($list, $item) {
+  $data = entity_metadata_wrapper($type, $args['settings']['value']);
+  return array('variable_added' => $data);

Primitive data types aren't wrapped. Thus just return the data, rules is going to wrap it if necessary.

+++ rules/modules/data.rules.inc
@@ -232,6 +233,63 @@ function rules_data_action_info() {
+       // Further needed parameters depend on the type.

Why not just add the value parameter here?

+++ rules/modules/data.rules.inc
@@ -532,6 +590,87 @@ function rules_action_data_list_add_positions() {
+function rules_data_action_variable_add_options() {
+  return _rules_data_variable_options('input form');
+}

We do not need to filter for input forms here. Data selection is fine for adding a variable.

+++ rules/modules/data.rules.inc
@@ -532,6 +590,87 @@ function rules_action_data_list_add_positions() {
+  foreach ($data_info as $type => $properties) {
+    if ($filter == 'creation callback' && isset($properties['creation callback'])) {
+      // Add data types with creation callback.
+      $options[$type] = $properties['label'];
+    }

You might want to use rules_filter_array() + rules_extract_property() here.

+++ rules/modules/data.rules.inc
@@ -532,6 +590,87 @@ function rules_action_data_list_add_positions() {
+  if (in_array('RulesDataDirectInputFormInterface', class_implements($class))) {
+    $element_info['parameter']['value'] = $type_info;
+    $element_info['parameter']['value']['type'] = $type;
+  }

We need the value parameter with all possible input modes. This process callback just needs to set the right data type.

Powered by Dreditor.

I think we are almost there now :)

+++ rules/modules/data.rules.inc
@@ -232,6 +233,66 @@ function rules_data_action_info() {
+      ),
+

Unnecessary line break.

+++ rules/modules/data.rules.inc
@@ -378,16 +439,17 @@ function rules_action_entity_query_process(RulesAbstractPlugin $element) {
+function rules_action_create_validate($element) {

Let's call that _type_validate like the form_alter, then the function naming is consistent.

+++ rules/rules.api.php
@@ -328,6 +328,10 @@ function hook_rules_event_info() {
+ *   - 'creation callback': Optionally, a callback that creates a new instance of
+ *     this data type. Entities should use hook_entity_info() to specify
+ *     'creation callback' instead, as introduced by the entity metadata
+ *     module. See rules_action_data_create_array() for an example.

Let's move this to the entry for 'property info' and explain it is only for data types with property info...

Perhaps:
If 'property info' is given, an optional callback that makes use of the property info to create ...

Powered by Dreditor.

Awesome work! and thanks for your endurance ;)

Committed.