Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow setting metadata to use the table option. Update example setting default_invoice_page. #16903

Merged
merged 1 commit into from
Apr 9, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Mar 26, 2020

Overview

A setting may specify a list of acceptable options (pseudoconstant). This PR adds another mechanism (table) for defining that list, and it specifically updates one setting (default_invoice_page) to use the new mechanism.

The mechanism matches an equivalent mechanism available in DAO fields.

See also, developer documentation: civicrm/civicrm-dev-docs#784

Before

The setting default_invoice_page in CiviContribute displays a list of contribution pages:

Screen Shot 2020-03-26 at 3 55 23 PM

The list is generated by a callback function (which performs the DB queries).

After

The setting default_invoice_page in CiviContribute settings displays the same list of contribution pages.

However, the callback metadata is replaced with table metadata.

Technical Details

This change will reduce the temptation to call silly core functions from extensions....

Comments

Builds off #16902

@civibot
Copy link

civibot bot commented Mar 26, 2020

(Standard links)

@civibot civibot bot added the master label Mar 26, 2020
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-dev-docs that referenced this pull request Mar 26, 2020
@eileenmcnaughton
Copy link
Contributor Author

test this please

@seamuslee001
Copy link
Contributor

@eileenmcnaughton can you rebase please

This extends the ways in which a pseudoconstant can be defined in a setting to better reflect the  ways that
work for the DAO objects.

In this one field is converted - default_invoice_page under CiviContribute settings. (Described as
Default invoice payment page)

The expected result is that the options in that page load the same as before.

This change will reduce the temptation to call silly core functions from extensions....
@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 rebased

@seamuslee001
Copy link
Contributor

@eileenmcnaughton I'm going to say the test fails relate

@eileenmcnaughton
Copy link
Contributor Author

test this please

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 interestingly the same errors were present on https://github.com/civicrm/civicrm-core/pull/16885/files. I didn't manage to reproduce locally so I'm not too sure what's going on

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 looks like it wasn't this PR!

@homotechsual
Copy link
Contributor

docs PR for this is approved - please adjust the labels if you can when this is merged - and assign the appropriate CiviCRM Core v5.2x milestone (create if needed!)

@totten totten changed the title Add more support for defining a pseudoconstant for a setting Allow setting metadata to use the table option. Update example setting default_invoice_page. Apr 3, 2020
@seamuslee001
Copy link
Contributor

(CiviCRM Review Template WORD-1.2)

@seamuslee001 seamuslee001 merged commit 0e815c9 into civicrm:master Apr 9, 2020
@seamuslee001 seamuslee001 deleted the settting_better branch April 9, 2020 01:35
homotechsual added a commit to civicrm/civicrm-dev-docs that referenced this pull request May 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants