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

Fix bug on handling 'is_required' #16901

Merged
merged 1 commit into from
Apr 6, 2020
Merged

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Mar 26, 2020

Overview

Fixes a bug that affects extensions using the generic form to render non-required select fields

Before

** (this is a patched version of @aydun's extension with fixes to comply better with more recent code changes - I will submit a PR back to the copy @aydun maintains once it is in gitlab or github)

After

'none' loads as an option for the group field on the settings page

Technical Details

The code was expecting 'Select' but we encourage 'select' in the settings spec

However, we DO know the case to expect for the quickFormType - so check that instead

Comments

@mattwire maybe one you have good familiarity with

The code was expecting 'Select' but we encourage 'select' in the settings spec

However, we DO know the case to expect for the quickFormType - so check that instead
@civibot
Copy link

civibot bot commented Mar 26, 2020

(Standard links)

@civibot civibot bot added the master label Mar 26, 2020
@eileenmcnaughton
Copy link
Contributor Author

test this please

@seamuslee001
Copy link
Contributor

I'm a +1 on this but will let matt comment as well

@eileenmcnaughton
Copy link
Contributor Author

test this please

@yashodha
Copy link
Contributor

yashodha commented Apr 6, 2020

@eileenmcnaughton tests all pass, ready to merge?

@mattwire mattwire merged commit bb454cb into civicrm:master Apr 6, 2020
@eileenmcnaughton eileenmcnaughton deleted the settings branch April 6, 2020 20:18
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