Skip to content

Fix entityname processing such that ContributionRecur tokens work; plus tests

Rich requested to merge artfulrobot/emailapi:artfulrobot-fix-entitynames into master

ContributionRecur tokens passed to Email.send via the CiviRules Action were broken because of the way entity names are handled and mangled throughout the codepath.

The problem was: ContributionRecurcontributionrecur_idcontributionrecurId (contributionrecur is not picked up by token processor which expects contribution_recur) and was created by an overly simple lower-casing of entity names.

The bug would be affecting any non-single-word entity, but this might in practise just mean ContributionRecur as I'm not sure many others are in use.

This PR fixes this while also introducing some new tests to prove it works and prove that the change has not affected other tokens that worked already.

I found this PR difficult because:

  • Civi is littered with inconsistencies in naming. I believe, for core at least, the preferred way to name an ID is fooID not foo_id and not fooId, at least for variable names. In API speak, entity names use CamelCase, API4 options are supposed to use camelCase and database fields/API4 non-option parameters are supposed to use snake_case. However, tokenprocessor requires (a) underscores between words in entity names and (b) Id suffix!

  • This extension introduces a way to pass context entities into its Email.Send API3 method with certain unique requirements; I couldn't find documentation on these.

  • It's CiviRules Action tries to map CiviRules' own unique data storage of context entities onto this extension's requirements. The result is that the original array of data gets transformed in two stages before it gets to the token processor.

  • No phpunit tests exist for CiviRules itself (as far as I can see?) so simulating a CiviRule Action invocation was a lot of work.

As well as fixing the problem I had (can't use ContributionRecur tokens) I've tried to address some of this by way of adding a lot of code commentry about what the code is doing, and why it's done a certain way.

Edited by Rich

Merge request reports

Loading