Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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
Comment | File | Size | Author |
---|---|---|---|
#21 | 1301022-rules-convert-without-round.patch | 3.35 KB | klausi |
#21 | 1301022-interdiff.patch | 2.56 KB | klausi |
#19 | rules_convert_data_type.patch | 4.58 KB | fago |
#9 | convert_data_action-1301022-9.patch | 7.86 KB | joelstein |
#8 | data_convert_action-1301022-5263490.patch | 7.47 KB | liupascal |
Comments
Comment #1
fagoyes, 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?
Comment #2
valante CreditAttribution: valante commentedSince 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?
Comment #3
valante CreditAttribution: valante commentedAt the risk of sounding rude or demanding (I apologize if that's case), could you please give a time estimate for this feature? Thanks!
Comment #4
pcambraMaybe also decimal/int/text to boolean.
Comment #5
liupascal CreditAttribution: liupascal commentedI 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 ?
Comment #6
pcambra@liupascal I'd say you can attach a patch here to start reviewing and completing it
Comment #7
liupascal CreditAttribution: liupascal commentedI 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)
Comment #8
liupascal CreditAttribution: liupascal commentedSorry i picked the wrong patch file ...
Comment #9
joelstein CreditAttribution: joelstein commentedI added the to/from Text option, to convert to/from strings.
Comment #10
geek-merlinthis 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.
Comment #11
mitchell CreditAttribution: mitchell commentedRe #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
Comment #12
geek-merlincode can be taken from dup #1479526: Action: modulus, ceiling, floor (rounding)
Comment #13
geek-merlinComment #14
fagoThanks!
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:
Watch out for the coding style. Missing space after // and missing trailing point.
No need to repeat what the code says anyway.
This seems to be unnecessary?
Let's improve this.
Let's use the new 'default mode' => 'selector' key.
Let's use "Integer" instead of "Int". Also, I don't think we need to capitalize Int and Decimal.
I don't think that's necessary here. NULL is fine for optional anyway.
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).
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.
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.
Removing the rounding behavior parameter should be done in info_alter.
That's done by info_alter - no need to do it on the form. Just make sure to reload the form.
Comment #15
mitchell CreditAttribution: mitchell commentedThanks, 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?
Comment #16
geek-merlin> 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...
Comment #17
mitchell CreditAttribution: mitchell commentedHere are some conversions that don't need rounding.
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.
Comment #18
mitchell CreditAttribution: mitchell commentedI 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.
Comment #19
fagoA 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
Comment #20
klausiThese constants are PHP 5.3 specific and cannot be used in a PHP 5.2 environment.
Comment #21
klausiHere 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.
Comment #22
fagoouch. 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.
Comment #23
fagoI'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.
Comment #24
geek-merlinfago, you rock. can't await to check it out.
Comment #25
Anonymous (not verified) CreditAttribution: Anonymous commentedI 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.
Comment #26
Anonymous (not verified) CreditAttribution: Anonymous commentedSetting "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.
Comment #27
geek-merlinad #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.
Comment #28
fagoWell, 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.
Comment #29
Anonymous (not verified) CreditAttribution: Anonymous commented@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.
Comment #30
geek-merlinso 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).
Comment #31
ghilsman CreditAttribution: ghilsman commentedWhat 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?
Comment #32
ghilsman CreditAttribution: ghilsman commentedNevermind, I put it in a module. "Cut & Paste" and I appreciate the code in the comments!!!
Comment #33
jainparidhi CreditAttribution: jainparidhi commented#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.
Comment #34
hgurol CreditAttribution: hgurol commentedCan 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)
Comment #36
Yuri CreditAttribution: Yuri commentedIs there a working patch available yet?
Comment #37
Yuri CreditAttribution: Yuri commentedPatch #21 applied manually, and it works, as is. Thanks.