Hi,

I desperately need some means of assigning values from decimal fields to integer fields. I use the PHP evaluation to round those numbers before passing them on to the integer fields, but the rules system still doesn't allow it.

Use case: Assigning User Points (integer) according to Line Item total (decimal).

May I suggest allowing decimal to integer assignation, perhaps while providing a warning that non-integer values will be rounded? Or at least to evaluate the PHP field and see if it returns an integer, and allow that?

Thanks,
Tal

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fago’s picture

Title: Converting Decimals to Integers » Add a type conversion action
Version: 7.x-2.0-rc2 » 7.x-2.x-dev
Component: Rules Core » Rules Engine

yes, we should add a type conversion action. Maybe call it "Convert data type" ?

input: old-var
output: converted-var

Type conversions:
decimal to int
decimal/int to date/duration
text to decimal/int/date/duration/url/token

Other conversions I'm missing?
Should we handle list of values too?

valante’s picture

Since it would take some awkward Ruling to apply this action to lists (create new list, loop over old, for each item convert, add new data item to new list), maybe a single action for lists is a good idea.

For me the big question is this: allow implicit conversion? In other words, should the user be allowed to assign a decimal value to an integer field, as long as s/he's warned that a conversion is taking place and some precision might be lost?

Which can possibly be boiled down to: should Rules conversions follow PHP conversions, or be stricter?

valante’s picture

At the risk of sounding rude or demanding (I apologize if that's case), could you please give a time estimate for this feature? Thanks!

pcambra’s picture

Maybe also decimal/int/text to boolean.

liupascal’s picture

I have a working action for converting decimal -> integer and integer -> decimal. It can serve as a starting "function" to handle more conversion type.
How can i share it ? Sandbox or patch on rules ?

pcambra’s picture

@liupascal I'd say you can attach a patch here to start reviewing and completing it

liupascal’s picture

I checked out "git checkout remotes/origin/7.x-2.x" as it looked like to be the dev branch ? (i don't know actually but the latest commit seemed to match)

liupascal’s picture

Status: Active » Needs review
FileSize
7.47 KB

Sorry i picked the wrong patch file ...

joelstein’s picture

I added the to/from Text option, to convert to/from strings.

geek-merlin’s picture

Status: Needs review » Reviewed & tested by the community

this works as expected.
reviewd the code: also everything as expected.

i'd like to extend this feature request:
for float->int conversion we have now the options to "round the half up/down".
we might want the option "round always up/down" and use the floor()/ceil() functions to do that.

but until someone(tm) finds time for that we should not block that useful piece of code from going in.

mitchell’s picture

Issue tags: +data transforms

Re #10, "extend[ing] this feature request": See #1479526: Action: modulus, ceiling, floor (rounding).

All: Please also contribute to #745314: Action: transform text to see this code come together.

edit: 'this code' link

geek-merlin’s picture

geek-merlin’s picture

Title: Add a type conversion action » Add number type conversion and rounding
fago’s picture

Status: Reviewed & tested by the community » Needs work

Thanks!

I had a look at the patch. First, let's start with the bird-eyes-view:

a) I think it makes more sense to have the user to choose the target-type first.

b) Once we have a target-type we know the set of possible source types, which we could just pass on as value for the source values's 'type' key. That + changing the description of it to mention possible source data types should already do it - so we can completely save the step of specifying the "from data type".

c) It would be nice to support parsing dates too, but that can be a follow-up.

d) Rules has test-coverage for all its conditions and actions. Thus, we'll need to add tests here too. Just add it to RulesIntegrationTestCase::testDataIntegration().

Ok, then let's look at the code:

+++ b/modules/data.eval.inc
@@ -169,6 +169,63 @@ function rules_action_variable_add_info_alter(&$element_info, RulesAbstractPlugi
+          //First apply the rounding behavior

Watch out for the coding style. Missing space after // and missing trailing point.

+++ b/modules/data.eval.inc
@@ -169,6 +169,63 @@ function rules_action_variable_add_info_alter(&$element_info, RulesAbstractPlugi
+          //Convert the value to int

No need to repeat what the code says anyway.

+++ b/modules/data.eval.inc
@@ -169,6 +169,63 @@ function rules_action_variable_add_info_alter(&$element_info, RulesAbstractPlugi
+    $cache = rules_get_cache();
+    $type_info = $cache['data_info'][$to_type];

This seems to be unnecessary?

+++ b/modules/data.rules.inc
@@ -192,10 +192,162 @@ function rules_data_action_info() {
+    'label' => t('Convert data type'),

Let's improve this.

+++ b/modules/data.rules.inc
@@ -192,10 +192,162 @@ function rules_data_action_info() {
+      'input' => array(
+        'type' => array('decimal', 'integer', 'text'),
+        'label' => t('Value to convert'),
+        'description' => t('The value to convert.'),
+      ),

Let's use the new 'default mode' => 'selector' key.

+++ b/modules/data.rules.inc
@@ -192,10 +192,162 @@ function rules_data_action_info() {
+        'label' => t('Rounding behavior (for Decimal value to Int conversion)'),

Let's use "Integer" instead of "Int". Also, I don't think we need to capitalize Int and Decimal.

+++ b/modules/data.rules.inc
@@ -192,10 +192,162 @@ function rules_data_action_info() {
+        'allow null' => TRUE,

I don't think that's necessary here. NULL is fine for optional anyway.

+++ b/modules/data.rules.inc
@@ -192,10 +192,162 @@ function rules_data_action_info() {
+function rules_action_data_convert_validate(RulesAbstractPlugin $element) {
+  if (!isset($element->settings['from_type'])) {
+    throw new RulesIntegrityException(t('"From" type is empty.'), array($element, 'parameter', 'from_type'));
+  }
+  if (!isset($element->settings['to_type'])) {
+    throw new RulesIntegrityException(t('"To" type is empty.'), array($element, 'parameter', 'to_type'));
+  }
+}

This checks are done by Rules anyway, so we can remove that validation part here. (Related checks have been recently removed for the data_is condition too).

+++ b/modules/data.rules.inc
@@ -192,10 +192,162 @@ function rules_data_action_info() {
+    '' => '- Select -',

We should look into adding this automatically once a parameter is optional. That can be a separate issue though. Maybe we should add at least a todo comment.

+++ b/modules/data.rules.inc
@@ -192,10 +192,162 @@ function rules_data_action_info() {
+  if(empty($element->settings['from_type'])) {
+++ b/modules/data.eval.inc
@@ -169,6 +169,63 @@ function rules_action_variable_add_info_alter(&$element_info, RulesAbstractPlugi
+    case 'text':
+      switch ($to_type) {
+        case 'integer':
+          $result = intval($input);
+          break;

I guess text-to-integer should also support rounding modes. So maybe go for floatval + the regular float to int conversion. Basically I think it's best to do it that way for all to-integer conversions.

+++ b/modules/data.rules.inc
@@ -192,10 +192,162 @@ function rules_data_action_info() {
+          if($key == 'rounding_behavior' && $element->settings['from_type'] == 'decimal')
+            continue;
+          unset($form['parameter'][$key]);

Removing the rounding behavior parameter should be done in info_alter.

+++ b/modules/data.rules.inc
@@ -192,10 +192,162 @@ function rules_data_action_info() {
+      //Force the input parameter field to the "from" data type
+      $form['parameter']['input']['settings']['help']['#parameter']['type'] = $element->settings['from_type'];

That's done by info_alter - no need to do it on the form. Just make sure to reload the form.

mitchell’s picture

Title: Add number type conversion and rounding » Action: data type conversion

Thanks, axel!

I read over your patch to sort out the differences between it and #1479526: Action: modulus, ceiling, floor (rounding). You marked it as a duplicate of this one, but I think they are different and the use cases are a bit different, too.

The other rounding functions are suited for list operations, and this type of rounding is suited for a single value. These rounding options are still useful, but they would be more useful as separate actions. If they were separate, you could call one of these rounding functions before the data type conversion or in other circumstances. What do you think about this?

This patch appears most focused on data type conversion. You even included a string value to number conversion, going beyond your initial requirements. So, to me, this patch is about number/integer conversion, not rounding, and also a starting point for other operations like these. What do you think?

geek-merlin’s picture

> If they were separate, you could call one of these rounding functions before the data type conversion or in other circumstances. What do you think about this?

hmm, i don't like cluttering action space.
decision is up to @fago enyway.

>So, to me, this patch is about number/integer conversion, not rounding, and also a starting point for other operations like these. What do you think?

in my point of view converstion does not make sense without a conversion strategy.
so it's
1. specify rounding
2. specify target type
ymmv...

mitchell’s picture

Title: Action: data type conversion » Action: type conversion for numbers, integers, and strings

in my point of view conversion does not make sense without a conversion strategy.
so it's
1. specify rounding
2. specify target type
ymmv...

Here are some conversions that don't need rounding.

  • Convert string "thirty" to an integer "30"
  • Convert decimal "10" to integer "10"
  • And those the other way

To me, rounding seems like it would be useful, on its own, in a variety of circumstances without type conversion.

Perhaps with a validation check and warning if the incoming data doesn't support an operation, it would all be kosher.

mitchell’s picture

I misread some of the code and thought it did text2int & int2text, in addition to decimal2text.
Here are examples:

But overall,decimal to integer is the most important one over each of the different types of number conversions. See #1525596: [Meta] Rules loops extended.

fago’s picture

A separate rounding action would be fine too, however still I'd agree this should be built-in as it's natural to configure it when converting to int.

So attached patch implements my suggestions from above. Please test.
TODO:
* Improve the data selector to add a help text that shows which data types are compatible, e.g. add a line that says "Valid data types: ".
* Add the "- select -" option once something is optional + null is allowed.
* Tests

klausi’s picture

+++ b/modules/data.rules.inc
@@ -192,10 +192,72 @@ function rules_data_action_info() {
+  return array(
+    PHP_ROUND_HALF_UP => t('Round half up (9.5 -> 10)'),
+    PHP_ROUND_HALF_DOWN => t('Round half down (9.5 -> 9)'),
+    PHP_ROUND_HALF_EVEN => t('Round half even (9.5 -> 10)'),
+    PHP_ROUND_HALF_ODD => t('Round half odd (9.5 -> 9)'),
+  );

These constants are PHP 5.3 specific and cannot be used in a PHP 5.2 environment.

klausi’s picture

Here is a patch for my drush make file that just kicks out the rounding behavior altogether for now. Feel free to work on it so that it is PHP 5.2 compatible. Interdiff is against #19.

fago’s picture

ouch. yep, it's not only the constants it's the whole operator. Maybe best lest just go with custom floor()/round(halfup)/ceil() support then.

fago’s picture

Status: Needs work » Fixed

I've implemented custom floor()/round(halfup)/ceil() support, added tests + committed it. As follow-up I've also added a help text that shows selectable data types during data selection.

geek-merlin’s picture

fago, you rock. can't await to check it out.

Anonymous’s picture

Status: Fixed » Active

I think one aspect of this feature is not handled properly.

Converting works for populated fields only. It should work for every field, even if the node yet doesn't provide data for it.

I.E.: When a node type is extended with a new field, all old nodes won't be updated and remain without an entry in the corresponding field table. Converting these not yet populated fields to numbers should return a default value (e.g.: zero or the default value of the field), but in any case return a variable of the requested type.

intval(null)===0 is familiar to many Drupal users. Type casting is done this way in PHP, but also in many other programming languages.

Current situation is: the conversion fails, the resulting variables remain unset and all subsequent actions on those variables fail, too.

Anonymous’s picture

Setting "allow null" (data_rules.inc @205) to true helps. As simple as it is. But of course with no respect to the configured default value of the custom field.

geek-merlin’s picture

ad #26: if i get it right the default value comes from the form layer.
so if a field is empty (and created by the ui ;-), it is allowed to be empty.

so i suppose "allow null" is just what we need.

fago’s picture

Converting these not yet populated fields to numbers should return a default value (e.g.: zero or the default value of the field), but in any case return a variable of the requested type.

Well, the value of the field is NULL. Why should a data type conversion make it to something not NULL? But enabling "allow null" and passing NULL through makes sense to me.

Anonymous’s picture

@axel.rutz
I thought the default value comes from the fields config, but you may be right that it usually is used at form level.

@fago
It's a conversion to a type. NULL has no type. Why do you want to pass through NULL? Can it be handled with other actions or will it cause other actions to fail? My problem is, the conversion action is not reliable in returning a variable of the requested type. This causes the whole rule to interrupt.

Actually it's a flaw in Drupal's core: to allow adding fields, but don't populate them to the loaded entities properly. Even if the field is required, it is not populated to old nodes. It would be hell to add new fields to old nodes from a performance perspective, and thus evident why they don't do that. I just wanted to mention.

geek-merlin’s picture

so the maximum feature wish would be an option like:

in case of null field values
1) pass through null
2) convert to zero
3) set to field default and convert

BUT i agree that just implementing 2) seems good for any practical use.

i support the idea "a conversion to number should always return a number" (ie not null).

ghilsman’s picture

What are the implications of hacking the patch from comment #8 above in to my sites 7.x-2.1 version in the data.eval.inc file? Will this work by just adding the 'Action: Convert a value' code into the data.eval.inc file?

ghilsman’s picture

Nevermind, I put it in a module. "Cut & Paste" and I appreciate the code in the comments!!!

jainparidhi’s picture

Status: Active » Needs review

#8: data_convert_action-1301022-5263490.patch queued for re-testing.

Sorry! Newbie here. Clicked on Test by mistake!

Edit: I successfully installed the patch at #8 but I realized that the patch does not convert Date into String! I tried installing patch at #21 but I am getting errors... Is it because its trying to replace the same code?

Thank you and I apologize for the inconvenience caused due to changing #8 status to Re-test.

hgurol’s picture

Can we make rounding a stand-alone action, or a part of a calculate a value action? Right now its only a part of data conversion and I want to use it without converting.

Also, I have noticed a weak wording about the rounding up.
It says Always up (9.5 -> 10)
I believe what Always up means (9.4 -> 10)

The last submitted patch, 1301022-interdiff.patch, failed testing.

Yuri’s picture

Issue summary: View changes

Is there a working patch available yet?

Yuri’s picture

Patch #21 applied manually, and it works, as is. Thanks.