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

Display Custom Fields on ContributionPage settings #31487

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

aydun
Copy link
Contributor

@aydun aydun commented Nov 15, 2024

Overview

This adds the display of Custom Fields on ContributionPages.

Before

Custom Fields on ContributionPages can be configured by API but not displayed.

After

Custom Fields on ContributionPages are displayed at the bottom of the Settings tab.

image

Technical Details

Comments

The customDataBlock.tpl that pulls in custom fields only loads groups with style of 'Inline'.

The GDPR extension creates custom fields on ContributionPages with Inline style but has its own code to display these as a tab. This PR causes those fields to also be shown on the Settings tab. This can be avoided by changing the style to Tab - I will create a PR for GDPR.

Copy link

civibot bot commented Nov 15, 2024

🤖 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...

➡️ Online demo of this PR 🔗

@civibot civibot bot added the master label Nov 15, 2024
@demeritcowboy
Copy link
Contributor

Looks good. Putting merge-ready just because it's not clear if it's intentional/out-of-scope that it doesn't work with subtypes, e.g. a custom group that is only for ContributionPages with financial type Donation. You can define such a custom group, but then it doesn't display here. I think it's ok for this if it's out-of-scope.

Test fails are also happening elsewhere.

@demeritcowboy demeritcowboy added the merge ready PR will be merged after a few days if there are no objections label Nov 16, 2024
@seamuslee001
Copy link
Contributor

Whilst I agree with this work, I think that we may want to think about some kind of upgrade step for people with this extension https://lab.civicrm.org/extensions/contributionpagecustomfield

@seamuslee001
Copy link
Contributor

Just to clarify the extension I was referring to above currently handles the ability to have custom fields on contribution pages and also displays them. So if we merge this then people upgrading would probably end up possibly with the custom data displaying twice right? So I was thinking we could essentially pull this option value into core https://lab.civicrm.org/extensions/contributionpagecustomfield/-/blob/master/contributionpagecustomfield.php?ref_type=heads#L42 and then effectively deprecate that whole extension at that point and have an upgrade message of like "We see that you had the Contribution Page Custom Field extension enabled, this has now been disabled as it is now incorporated in core or similar"

@colemanw
Copy link
Member

colemanw commented Nov 18, 2024

@seamuslee001 actually that wheel has already been invented :)
Just push an entry into this file (could do it as part of this PR for completeness): https://github.com/civicrm/civicrm-core/blob/master/extension-compatibility.json

@aydun
Copy link
Contributor Author

aydun commented Nov 19, 2024

Looks good. Putting merge-ready just because it's not clear if it's intentional/out-of-scope that it doesn't work with subtypes, e.g. a custom group that is only for ContributionPages with financial type Donation. You can define such a custom group, but then it doesn't display here. I think it's ok for this if it's out-of-scope.

Test fails are also happening elsewhere.

@demeritcowboy Thanks - It's kinda out of scope in that I didn't know you could do that and don't need it. But I'm happy to try putting that in for completeness. However, the Custom Groups UI does not provide that option and I don't see how to do it with APIv4 Explorer. Could you provide more detail please?

@demeritcowboy
Copy link
Contributor

It's option_value.grouping. See https://docs.civicrm.org/dev/en/latest/step-by-step/create-entity/#121-making-our-entity-available-for-custom-data

And then the {include} in the tpl has a parameter customDataSubType. I don't know how magic it is - if it can handle anything you throw at it.

@demeritcowboy
Copy link
Contributor

Partly what I mean by "magic" is that if for example it doesn't add an onChange handler on the financial_type_id field, or whatever field the user happens to put in grouping, that then rebuilds the custom data, then it wouldn't be properly functional anyway.

@aydun
Copy link
Contributor Author

aydun commented Nov 22, 2024

@demeritcowboy - I think that now handles financial_type subtypes.

@aydun
Copy link
Contributor Author

aydun commented Nov 22, 2024

@seamuslee001 @colemanw I've enabled custom fields on ContributionPage in core, and updated extension-compatibility.json to obsolete the contributionpagecustomfield extension.

FYI, if you're testing, this needs to be applied to make the contributionpagecustomfield extension work on master (prior to this PR).

I think that covers everything ...

@demeritcowboy demeritcowboy removed the merge ready PR will be merged after a few days if there are no objections label Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants