[NFC] SearchKit sql function "if": fix description (dev/core#4307) #26333
Conversation
|
(Standard links)
|
Civi/Api4/Query/SqlFunctionIF.php
Outdated
| @@ -51,7 +51,9 @@ public static function getTitle(): string { | |||
| * @return string | |||
| */ | |||
| public static function getDescription(): string { | |||
| return ts('If the field is empty, the first value, otherwise the second.'); | |||
| return ts('If the field is boolean TRUE, or any number except 0, or a ' | |||
There was a problem hiding this comment.
String concatenation inside the ts() isn't allowed, so I think this needs to be all one line.
There was a problem hiding this comment.
Also, I think this isn't consistent with the results in your issue, where "my_1_and_only" returns FALSE and "5_golden_rings" returns TRUE (so I think it would actually be a string starting with 1-9 gives the first value, any other string the second).
But I wonder if it really makes sense to have IF() be an option for any field other than booleans and maybe integers? Behaviour with strings seems inconsistent and not helpful (I can't imagine a case where you would want a non-empty string to return FALSE).
There was a problem hiding this comment.
String concatenation inside the ts() isn't allowed, so I think this needs to be all one line.
How would ts() know the difference between 'xy' and 'x' . 'y'? They're exactly the same by the time the function sees them.
Also, I think this isn't consistent with the results in your issue, where
"my_1_and_only"returnsFALSEand"5_golden_rings"returnsTRUE(so I think it would actually be a string starting with 1-9 gives the first value, any other string the second).
Good catch. Just fixed that.
I wonder if it really makes sense to have IF() be an option for any field other than booleans and maybe integers?
Good question, but I'm just trying to accurately document how it currently works, which as Coleman says is weird.
There was a problem hiding this comment.
I don't know the exact details of how the translation works, but I'm assuming it's because there's something that scans through all the code and pulls out everything inside a ts('') to add it to the list of strings to be translated. So then when someone goes to translate this, the string won't be right in the translation system (not sure how it breaks, either they might just get the bit enclosed in the first quotes or maybe the whole thing with line breaks and dots).
There was a problem hiding this comment.
How would ts() know the difference between 'xy' and 'x' . 'y'? They're exactly the same by the time the function sees them.
@highfalutin you are correct that string concatenation doesn't make a difference at runtime, but @larssandergreen is correct that the source code is scanned for translatable strings using a regex pattern that is not compatible with concatenation. See https://docs.civicrm.org/dev/en/latest/translation/#best-practices
There was a problem hiding this comment.
Ah ok, that makes more sense, thanks. I just put it all on one long line and rebased.
24c04f5 to
16bc299
Compare
Fixes dev/core#4307: incorrect description for sql function "If/Else" in SearchKit.