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

dev/core#2459 Fix custom fields changed from multiple-choice data type to Text #19794

Merged
merged 1 commit into from
Mar 16, 2021

Conversation

MegaphoneJon
Copy link
Contributor

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

Overview

Changing a custom field from a multiple choice (Radio, Checkboxes, etc.) to a free-entry text box works, until you try to write to the custom field via the API.

Reproduction steps are on the ticket.

Before

API Error - your value isn't an accepted value for this field.

After

A custom field with a data type of "String" and HTML type of "Text" should take any alphanumeric value.

Technical Details

Changing the HTML type doesn't remove the Option Group ID. While arguably the API code should ignore that, it seems more important that the data be correct.

@civibot
Copy link

civibot bot commented Mar 12, 2021

(Standard links)

@civibot civibot bot added the master label Mar 12, 2021
@@ -1994,6 +1994,11 @@ protected static function prepareCreate($params) {
}
}

// Remove option group IDs from fields changed to Text html_type.
if ($htmlType == 'Text') {
Copy link
Member

Choose a reason for hiding this comment

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

This is a good change. Are there other html types this should also apply to?

@eileenmcnaughton eileenmcnaughton changed the title Fix custom fields changed from multiple-choice data type to Text dev/core#2459 Fix custom fields changed from multiple-choice data type to Text Mar 16, 2021
@eileenmcnaughton
Copy link
Contributor

I'm going to merge this as it makes sense as is and I don't think it should be blocked on @colemanw's question about whether the scope is broad enough

@MegaphoneJon please consider the comment & if you still this goes far enough please close out the gitlab

@eileenmcnaughton eileenmcnaughton merged commit 5e09246 into civicrm:master Mar 16, 2021
@MegaphoneJon
Copy link
Contributor Author

@colemanw There aren't other HTML types where this should happen. Every data type that allows an option group (String, Int, Float, Money) has HTML types that require an option group or are "Text".

Now, if I ever get around to implementing that "Hidden" HTML type I've been toying with, that'd be a different story...

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