Skip to content
This repository has been archived by the owner on Oct 8, 2020. It is now read-only.

Add in notes about support for KeyColumn parameter when using option … #571

Merged

Conversation

seamuslee001
Copy link
Collaborator

…group as part of a setting pseudoconstant

ping @MegaphoneJon @totten this covers the changes for settings pseduoconstants as per civicrm/civicrm-core#13361

Copy link
Contributor

@MegaphoneJon MegaphoneJon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if we reference the DAOs we should either a) link to the relevant docs, or b) say specifically what values aren't supported. I'm not wedded to these changes but I did have a hard time parsing what the new functionality was. Thanks for documenting it though :)

@@ -63,7 +63,7 @@ The Supported Properties for settings are:
| `html_type` | Html type (admin form)| This is the preferred way to describe the html type as it is not quick form specific. It will be used it present. Syntax is lower case. e.g 'select', 'radio', 'checkboxes', 'checkbox', 'text', 'textarea', 'entity_reference|
| `quick_form_type` | Widget type (admin form)| YesNo, CheckBox, CheckBoxes, Select, EntityRef. This is not required if html_type (preferred) is set|
|`settings_pages`|Admin Pages to render this setting on|e.g ['event' => ['weight' => 10]]. This works if the Generic form is used (see further down)|
|`pseudoconstant`|Provides information to build a list of available options| This is the perferred methodology for lists of options and currently supports either a callback - e.g ```['callback' => 'CRM_Core_SelectValues::geoProvider']``` or an option group name [`'optionGroupName' => 'advanced_search_options'`]. It does not currently support the full range of values for this key that the DAO schema does, by ommission rather than design|
|`pseudoconstant`|Provides information to build a list of available options| This is the preferred methodology for lists of options and currently supports either a callback - e.g ```['callback' => 'CRM_Core_SelectValues::geoProvider']``` or an option group name [`'optionGroupName' => 'advanced_search_options'`]. As in the case of the DAOs you can also when specifying an `optionGroupName` you can specify `keyColumn` which specifies which column within `civicrm_option_value` to use as the key, by default this will be the value column. It does not currently support the full range of values for this key that the DAO schema does, by ommission rather than design|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
|`pseudoconstant`|Provides information to build a list of available options| This is the preferred methodology for lists of options and currently supports either a callback - e.g ```['callback' => 'CRM_Core_SelectValues::geoProvider']``` or an option group name [`'optionGroupName' => 'advanced_search_options'`]. As in the case of the DAOs you can also when specifying an `optionGroupName` you can specify `keyColumn` which specifies which column within `civicrm_option_value` to use as the key, by default this will be the value column. It does not currently support the full range of values for this key that the DAO schema does, by ommission rather than design|
|`pseudoconstant`|Provides information to build a list of available options| This is the preferred methodology for lists of options and currently supports either a callback - e.g ```['callback' => 'CRM_Core_SelectValues::geoProvider']``` or an option group name [`'optionGroupName' => 'advanced_search_options'`]. When specifying an `optionGroupName` you can optionally specify `keyColumn` to return a column from `civicrm_option_value` to use as the key. By default the `keyColumn` is the `value` column.|

@seamuslee001
Copy link
Collaborator Author

@MegaphoneJon ok i have updated this now i think it should meet your requirements

Copy link
Contributor

@MegaphoneJon MegaphoneJon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a lot easier to parse, thank you @seamuslee001! Just a minor fix so the link works and I think this is good to merge.

@@ -63,7 +63,7 @@ The Supported Properties for settings are:
| `html_type` | Html type (admin form)| This is the preferred way to describe the html type as it is not quick form specific. It will be used it present. Syntax is lower case. e.g 'select', 'radio', 'checkboxes', 'checkbox', 'text', 'textarea', 'entity_reference|
| `quick_form_type` | Widget type (admin form)| YesNo, CheckBox, CheckBoxes, Select, EntityRef. This is not required if html_type (preferred) is set|
|`settings_pages`|Admin Pages to render this setting on|e.g ['event' => ['weight' => 10]]. This works if the Generic form is used (see further down)|
|`pseudoconstant`|Provides information to build a list of available options| This is the perferred methodology for lists of options and currently supports either a callback - e.g ```['callback' => 'CRM_Core_SelectValues::geoProvider']``` or an option group name [`'optionGroupName' => 'advanced_search_options'`]. It does not currently support the full range of values for this key that the DAO schema does, by ommission rather than design|
|`pseudoconstant`|Provides information to build a list of available options| This is the preferred methodology for lists of options and currently supports either a callback - e.g ```['callback' => 'CRM_Core_SelectValues::geoProvider']``` or an option group name [`'optionGroupName' => 'advanced_search_options'`]. When specifying an `optionGroupName` you can optionally specify `keyColumn` to return a column from `civicrm_option_value` to use as the key. By default the `keyColumn` is the `value` column. The format is the same as that used for [DAO](/framework/database/schema-definition.md#table-field-pseudoconstant)|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
|`pseudoconstant`|Provides information to build a list of available options| This is the preferred methodology for lists of options and currently supports either a callback - e.g ```['callback' => 'CRM_Core_SelectValues::geoProvider']``` or an option group name [`'optionGroupName' => 'advanced_search_options'`]. When specifying an `optionGroupName` you can optionally specify `keyColumn` to return a column from `civicrm_option_value` to use as the key. By default the `keyColumn` is the `value` column. The format is the same as that used for [DAO](/framework/database/schema-definition.md#table-field-pseudoconstant)|
|`pseudoconstant`|Provides information to build a list of available options| This is the preferred methodology for lists of options and currently supports either a callback - e.g ```['callback' => 'CRM_Core_SelectValues::geoProvider']``` or an option group name [`'optionGroupName' => 'advanced_search_options'`]. When specifying an `optionGroupName` you can optionally specify `keyColumn` to return a column from `civicrm_option_value` to use as the key. By default the `keyColumn` is the `value` column. The format is the same as that used for [DAO](/framework/database/schema-definition/#table-field-pseudoconstant)|.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MegaphoneJon my understanding is that we are meant to use the .md extension in the internal linkages because markdown docs will detect if it breaks. As per https://docs.civicrm.org/dev/en/latest/documentation/markdown/#internal-url-standards

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Live and learn! I tested the link, it didn't work. But this does look better, yes.

@MegaphoneJon MegaphoneJon dismissed their stale review December 30, 2018 20:38

Live and learn

@MegaphoneJon MegaphoneJon merged commit 2306995 into civicrm:master Dec 30, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants