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#2772 - Don't crash for custom fields of type int that are multi-select #21186

Merged
merged 1 commit into from
Sep 7, 2021

Conversation

demeritcowboy
Copy link
Contributor

@demeritcowboy demeritcowboy commented Aug 19, 2021

Overview

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

Can't save a record that has a value entered into a custom field of type int which is a dropdown multi-select.

Before

  1. Create a custom field of type int.
  2. Make it a dropdown.
  3. Check the box for multiselect.
  4. Create a record and fill in a selection for the custom field.
  5. Save.

CRM_Core_Exception: One of parameters (value: ☐1☐) is not of the type Timestamp in .../CRM/Core/DAO.php on line 1713

After

Saves.

Technical Details

is_multiple is for the custom_group and whether it can have multiple instances of the group.
serialize is for the field and specifies multi-select.

Comments

This same issue was supposedly fixed in 5.35 (https://lab.civicrm.org/dev/core/-/issues/2423), but I've tested 5.35.2 and it doesn't work there either.

@civibot
Copy link

civibot bot commented Aug 19, 2021

(Standard links)

@civibot civibot bot added the master label Aug 19, 2021
@demeritcowboy
Copy link
Contributor Author

demeritcowboy commented Aug 20, 2021

I'm a little unsure what to do about the last set of fails. They're situations where CustomValueTable::store() has been called directly but without looking up the serialize value from the field definition. (And I haven't tested but I'd argue that the existing hiding missing is_multiple with ?? is wrong here - you want it to be whatever the group definition says, not arbitrarily pick single if you haven't specified.)

@colemanw
Copy link
Member

Maybe ?? CRM_Core_DAO::getFieldValue(...)

@demeritcowboy demeritcowboy changed the title [WIP] dev/core#2772 - Don't crash for custom fields of type int that are multi-select dev/core#2772 - Don't crash for custom fields of type int that are multi-select Aug 21, 2021
@colemanw
Copy link
Member

colemanw commented Sep 7, 2021

This looks great now and tests are happy with it too.

@colemanw colemanw merged commit 28e3220 into civicrm:master Sep 7, 2021
@demeritcowboy
Copy link
Contributor Author

Thanks!

@demeritcowboy demeritcowboy deleted the multiint branch September 7, 2021 13:35
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.

2 participants