search

progressivetech / net.ourpowerbase.sumfields

The Summary Fields extension creates read-only custom data fields that extend Contact and are automatically populated with up-to-date totals such as total lifetime contributions, last contribution amount, last attended event, etc.
Other
8 stars 29 forks source link

Crash when using "Revert" from advanced logs #99

Open MegaphoneJon opened 1 year ago

MegaphoneJon commented 1 year ago

I know this complicates an already complicated set of NULLIF and IFNULL, but when you try to use the Revert this Change button on the advanced Change Log tab, you get a crash.

You have some code that generates this as part of a trigger: 

 IFNULL(OLD.`amount_of_last_contribution_77`,'')

This places an empty string instead of a NULL, for understandable reasons.

However, these custom fields are of type "Money", and "Money" is validated with CRM_Utils_Rule::numeric(). That says only numeric values pass validation.

This normally doesn't come up because the triggers bypass validation, and the fields are read-only, so the value is never changed in a validated way - except if you try to revert the change.

I'm not sure it's worth the time it'll take to fix, but I wanted to make sure it's documented for others who experience this.

jmcclelland commented 1 year ago

Thanks for the report @MegaphoneJon . I'm struggling to figure this one out - when grepping the code, I only see "IFNULL" in custom.php in the event attended and event no show queries - but not in the amount of last contribution query, so I'm not really share how that query is created. Or is that the query execute by CiviCRM core when reverting?

Maybe it's not worth fixing as you suggest, but since we define each field with their type, it seems at least theoretically possible to change a '' to a 0 for Money types?