(dev/core#5614) Search Kit - Allow DB Entity with custom column names

[totten]

Overview

When creating a saved-search, one may define a "DB Entity" (eg SQL TABLE or VIEW derived from the query). This update allows the administrator to define the SQL column names.

(See also: https://lab.civicrm.org/dev/core/-/issues/5614)

Steps to Reproduce

• Create a new saved-search
• Choose some columns and criteria
• Add a search-display of type "DB Entity"
• Configure the list of columns for the entity

Before

For each column, you may define the visible label:

These labels will appear as the headings in downstream searches.

The SQL column-name is auto-generated from the original SELECT expression that provided the data. In simple scenarios, the generated name is intuitive. But in complex scenarios, the name becomes opaque (eg GROUP_CONCAT_Contact_RelationshipCache_Conc51a580ccf5b246a).

After

For each column, you may define both the SQL column-name and the visible label.

Technical Details

• Existing configurations for DB entities do not have an explicit name property -- they strictly rely on auto-generated column-names. For continuity, this data is treated the same as before. (A blank name means that the column-name will be auto-generated.)

• Additionally, this adds some guards to the admin UI -- if you try to rename an existing column, it will show warning like this:

  

  Similarly, if you try to delete a DB Entity, it will also show a warning:

  

[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 🔗

[herbdool]

I haven't tested but this appears to be exactly what I was imagining plus more.

[totten]

• Existing configurations for DB entities do not have an explicit name property -- they strictly rely on auto-generated column-names. For continuity, this data is treated the same as before. (A blank name means that the column-name will be auto-generated.)

I still think this is good in terms of handling upgrades safely.

However, there is a visual factor that would feel quirky (esp over time) -- in the UI, it defaults to an empty name for each column. It would feel more natural if the name UI was visibly prepopulated. And when doing prepopulation, there are couple a ways to go (and I'm not certain difficulty of either):

• New-columns only: Only prepopulate the name for new columns. Any existing columns remain as name="" (implying the use of the current/legacy formula).
  • On the plus side -- when iterating on the formula in the future, there's little risk. The formula only define a default suggestion. Once created, it is stored persistently. (We don't have to worry that future upgrades will flipflop on column names.)
  • On the down side -- existing columns still look quirky (name=""), and you may end-up with two formulas.(The "new column" formula vs the "legacy/upgrade" formula.)
• New-columns and pre-existing columns: Take the existing formula. Extend it to be apparent in the UI for any/all columns.
  • On the plus side -- this applies equally to new and pre-existing columns. There is only one formula.
  • On the down side -- The formula should basically be locked-down (no fiddling with magic-numbers, etc). Over time, the meaning of name="" will remain significant to upgrades. (It's hard to plot all the upgrade-paths... after considering the use mgd's for SK, paths through versions/releases, the circumstantialness in which items get manual edits, etc....) This will feel like a drag -- because as time goes on, the upgrades will be less in our minds-eye, and the behavior of new-fields will feel more relevant.

(In both cases, there's probably some consideration about how the JS+PHP code interact, though I'm not sure which variations are easy/hard.)

(EDIT) IMHO, the changes are safe regardless of whether we prepopulate names. It's a question of how much time/priority we want to put into the prepopulation (e.g. do it upfront vs do it in some other PR).

[totten]

Small thought: There are common critiques of confirmation dialogs -- they get annoying; people get de-sensitized; etc. That's well and good.

IMHO, this is consistent with that:

• The call to $ctrl.confirmSave() is a computation that decides whether a prompt is worthwhile.
• For most usual edits to regular saved-searches, it will proceed as directed by the user. No need to confirm.
• It only prompts for confirmation if you are deleting or renaming a schema-element (because other things might depend on the schema-element). And it specifically identifies the changed elements.

[colemanw]

@totten It would feel more natural if the name UI was visibly prepopulated. And when doing prepopulation, there are couple a ways to go (and I'm not certain difficulty of either):

I would vote for option 3: use the formula to set the placeholder attribute of the field. That way it can be left blank and the user will know the default value. This will apply equally well to new and existing displays.