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#1282 Takes care of customfields of type multiselect that were not being rendered #15375

Merged
merged 4 commits into from
Oct 28, 2019

Conversation

VangelisP
Copy link
Contributor

Overview

Customfields of type Multiselect attached to an Address do not render in profile page (appear empty). Related issue: https://lab.civicrm.org/dev/core/issues/1282

Before

MultiSelect customfield did not render values
before-patchfix

After

MultiSelect customfield does render values
after-patchfix

Technical Details

Examines (using switch) the html-type of the field to render, as already being done a few lines before: https://lab.civicrm.org/dev/core/blob/master/CRM/Core/BAO/UFGroup.php#L2373-2400
If of type:

Comments

It seems that Checkbox works by itself. I could minimize the check only to the Multi-Select elements.

@civibot
Copy link

civibot bot commented Oct 2, 2019

(Standard links)

@civibot civibot bot added the master label Oct 2, 2019
@eileenmcnaughton eileenmcnaughton changed the title Takes care of customfields of type multiselect that were not being rendered dev/core#1282 Takes care of customfields of type multiselect that were not being rendered Oct 7, 2019
@eileenmcnaughton
Copy link
Contributor

I feel like this is duplicating code & maybe we could extract & share it?

@VangelisP
Copy link
Contributor Author

I believe this is much better.

I've isolated : https://lab.civicrm.org/dev/core/blob/master/CRM/Core/BAO/UFGroup.php#L2369-2395 into its own function and also took out completely that part as I rely now on the variable $field to get my information from (instead of the old manual way).

I believe we might need some reviewers because for example I don't play with cases and I don't know if this generates any issue over the case component.

@VangelisP
Copy link
Contributor Author

I found out that if location type is Primary, select/multiselect would also not render. The latest commit fixes that as well

@eileenmcnaughton
Copy link
Contributor

I did some digging into the code & this seems to all make sense / simplify the code.

I note that lines were removed that related to https://issues.civicrm.org/jira/browse/CRM-2969 but I tried to test the scenario described there and it worked from what I could see - screen shot below.

Screen Shot 2019-10-28 at 10 18 05 PM

I'm still tinkering a bit more on tests but I did put one up #15638 - I think this is good to merge

@eileenmcnaughton eileenmcnaughton merged commit 6f2246e into civicrm:master Oct 28, 2019
@VangelisP VangelisP deleted the dev_issue#1282 branch November 2, 2019 08:24
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.

2 participants