core#2386 - metadata-driven chain-select fields

[MegaphoneJon]

https://lab.civicrm.org/dev/core/-/issues/2386

Overview

As we move to metadata-based settings, we need to support existing use cases such as chain-select.

Before

It looks like someone started writing code to allow defining chain-selects in *.setting.php but didn't finish.

After

Chain-selects work., we can create metadata-driven chain-select fields.

Technical Details

This adds a new property for settings, chain_select_properties. These values are passed to CRM_Core_Form::addChainSelect.

Comments

I'm not sure how tightly the properties of CRM_Core_Form::addChainSelect are coupled to Quickform. If they are, we may need a more generic solution - like a settings property that defines a JS function to run on an onclick event.

If this PR is accepted, I will add documentation in the Settings Reference.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[colemanw]

Thanks for this @MegaphoneJon I'm trying to understand how to test it. When I visit mysite.org/civicrm/admin/setting/localization?reset=1 I see a chainSelect for state province but it already works even without this PR. Is that because that form isn't metadata-based? And would it be difficult for you to switch that field over to using metadata so we can see a working example of what this PR does?

[MegaphoneJon]

@colemanw That makes a lot of sense, I'll do that.

[MegaphoneJon]

@colemanw I replaced templates/CRM/Admin/Form/Setting.tpl's contents with {include file="CRM/Form/basicFormFields.tpl"}. You're right, this field's chain-select does work, because of this bit of hackery that defines the control-field with some str_replace that a) prescribes your setting names, b) is tied to existing chain-selects.

I encountered this first in an extension where the name of my country/state fields don't line up this way - and I could just change the names, but it seems like there's a deeper need for chain-select that's decoupled from the existing country/state and state/county workflows. I'm OK with letting the scope creep here if we can achieve consensus on how to implement this.

A closely related issue is the ability to add an onclick event to settings to implement showing/hiding fields. So the scope might creep still more...

[MegaphoneJon]

Ah - the reason it works is also because quick_form_type is defined. If you rely solely on html_type you get a QuickForm error. I see in the Settings Reference that quick_form_type isn't deprecated yet - is that actually true? I thought html_type superseded it.

[colemanw]

Ah - the reason it works is also because quick_form_type is defined. If you rely solely on html_type you get a QuickForm error. I see in the Settings Reference that quick_form_type isn't deprecated yet - is that actually true? I thought html_type superseded it.

I think @eileenmcnaughton would know the answer to that.

[eileenmcnaughton]

@MegaphoneJon I think the comment is a deprecation of sorts but could be more explicit

https://docs.civicrm.org/dev/en/latest/framework/setting/

[MegaphoneJon]

@eileenmcnaughton I'll open a PR to move it into the "deprecated properties" section below then.

To recap here - there's no implementation of chainselect as an option for html_type, which we'll 100% need. But this implementation relies on CRM_Core_Form::addChainSelect(), and afform might not have a chain-select helper. It could use an (unimplemented) onclick property (a JS callback) and html_attributes.

We should have a sense of how we'll implement afform chain-select to ensure this doesn't lock in a bad contract.

[eileenmcnaughton]

I tested this and it works. I will put up an another PR that facilitates swapping out this field to use it without doing all the fields on the form

[colemanw]

afform might not have a chain-select helper

Afform does have a chain-select implementation already. You can see it in action by creating a simple Contact + Address form. It works via metadata.

[eileenmcnaughton]

#19697 convertst the field

@colemanw the metadata for the settings is a bit different - 'something' has to know the relation between country & state fields. Note that the metadata @MegaphoneJon added has done that

[colemanw]

Actually it's the same in Afform. You might want to verify that @MegaphoneJon's approach is consistent to Afform's, which is to use the APIv4 getFields metadata, which looks like this for the Address entity (the relevant bits anyway):

"county_id": {
    "label": "County",
    "input_type": "ChainSelect",
    "input_attrs": {
      "label": "County",
      "controlField": "state_province_id"
    }
  },
  "state_province_id": {
    "label": "State/Province",
    "input_type": "ChainSelect",
    "input_attrs": {
      "label": "State/Province",
      "controlField": "country_id"
    }
  },
  "country_id": {
    "label": "Country",
    "input_type": "Select",
    "input_attrs": {
      "label": "Country"
    }
  },

[eileenmcnaughton]

@MegaphoneJon ok it seems we need the to align with 'input_attrs' (because we love words like 'attrs' that presumably mean 'attributes' in a random sorta way)

[colemanw]

Also APIv4 uses camelCase for "controlField" although that appears to be a mistake. Everything returned by the API is supposed to use lowercase keys. Oops. I think that might be too late to fix now... or maybe not it's a pretty obscure feature. Maybe we should fix it now while we still can.

[eileenmcnaughton]

@colemanw I think we still can if we want to

[MegaphoneJon]

@MegaphoneJon ok it seems we need the to align with 'input_attrs' (because we love words like 'attrs' that presumably mean 'attributes' in a random sorta way)

@eileenmcnaughton I thought from this that I would need to rework this PR, but I see it's merged. Was that in error?

[eileenmcnaughton]

@MegaphoneJon yep - @colemanw helpfully pointed it out after I merged it

[eileenmcnaughton]

@MegaphoneJon you might pull in #19697 when youwork on it to check it works against that