CRM_Utils_String - Fix convertStringToSnakeCase to prevent double underscores

[colemanw]

Overview

Fixes a quirk in a string converter function, and adds a bunch of unit tests.

[civibot]

🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷

Introduction for new contributors...

• If this is your first PR, an admin will greenlight automated testing with the command ok to test or add to whitelist.
• A series of tests will automatically run. You can see the results at the bottom of this page (if there are any problems, it will include a link to see what went wrong).
• A demo site will be built where anyone can try out a version of CiviCRM that includes your changes.
• If this process needs to be repeated, an admin will issue the command test this please to rerun tests and build a new demo site.
• Before this PR can be merged, it needs to be reviewed. Please keep in mind that reviewers are volunteers, and their response time can vary from a few hours to a few weeks depending on their availability and their knowledge of this particular part of CiviCRM.
• A great way to speed up this process is to "trade reviews" with someone - find an open PR that you feel able to review, and leave a comment like "I'm reviewing this now, could you please review mine?" (include a link to yours). You don't have to wait for a response to get started (and you don't have to stop at one!) the more you review, the faster this process goes for everyone 😄
• To ensure that you are credited properly in the final release notes, please add yourself to contributor-key.yml
• For more information about contributing, see CONTRIBUTING.md.

Quick links for reviewers...

• ➡️ PR Demo site (can be rebuilt with the command test this please).
• 📖 Review standards
• 🗒️ Review template (brief or verbose)

➡️ Online demo of this PR 🔗

[totten]

Beautiful tests. :)

In terms of the function itself, it sounds pretty sensible. (I don't know the situation that's motivating, but it seems fair on its own.)

In terms of downstream effects, I tried grepping core for usages. Many callers look OK (e.g. they have narrow input domains or ephemeral usages). The question for consideration would be places where (1) the input-domain is wide and/or (2) the runtime-computed foo_bar_baz needs to match a stored foo__bar_baz.

Two that came up as possible concerns:

• Afform Tabs: FiveSixtySix::updateAfformTabs and afform_civicrm_tabset
  • Since this has an upgrader, it feels like there probably is some matching to stored data.
• Search Kit Tables: _getSearchKitDisplayTableName
  • This data is replaceable. My guess would be that an unmaged change would be disruptive but repairable.

However, @colemanw, you probably know the dataflows around those two better than me. I'll defer to you on how much change-management is necessary (e.g. none vs upgrade-warning vs upgrade-cleanup).

[colemanw]

Beautiful tests. :)

One of the first times I've used the AI assistant and it actually did something helpful. 2 out of the 3 tests it wrote passed without any tweaking.

Good catch grepping around, I think that:

1. FiveSixtySix::updateAfformTabs this upgrade step is safe to run multiple times, so let's just run it again. I've moved it to the latest upgrader. And while I was at it I noticed that it wasn't properly dealing with custom field group tabs, so we needed to run that upgrader again anyway.
2. afform_civicrm_tabset yea that's the motivator, it was giving weird tab names before.
3. _getSearchKitDisplayTableName: I agree the data is replaceable. I'm pretty sure that if the table can't be found (e.g. the generated name is different) then it will just be recreated.

[totten]

afform_civicrm_tabset: I tried to find the problem being fixed by playing around with a recent version, 5.81, and then (1) make a saved-search, (2) make a form with funny name, (3) add it to tabs, (4) tweak tabs in contactlayouteditor, (5) view the IDs of the resulting tabsets. I used various mixes of capital-letters and symbols, and couldn't find figure any input that would lead to double-underscores in tab IDs.

_getSearchKitDisplayTableName: Similarly, I tried making "DB Entities" with various wonky names (various mixes of capitals and symbols), and couldn't provoke double-scores in table-names.

So... I can't really confirm if there's a problem being fixed. But, on the plus side, those things still seemed to work the same after the patch...