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

Add DB default of 0 for custom_group.is_multiple - CRM-21853 #12116

Merged
merged 1 commit into from
May 14, 2018

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented May 11, 2018

Overview

Make the civicrm_custom_group.is_multiple field DB default = 0 (to resolve #11877). I also set is_active to default to 1 while I was at it as it seemed broadly consistent with other tables

Before

No default for field civicrm_custom_group.is_multiple

After

Default is 0

Technical Details

Tests should pass on #11877 after this & it should be mergeable

Comments

@mickadoo this is to try to resolve an older PR of yours. I thought about closing it but decided the missing piece of work was relatively trivial


@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 @monishdeb @colemanw is one of you up to review this - removes a blocker to getting an old PR by Compucorp merged (& that PR fixes a bug)

@seamuslee001
Copy link
Contributor

Changes look ok to me @eileenmcnaughton have you tested on different MySQL engines in case the SET command doesn't work on all versions?

@mickadoo
Copy link
Contributor

Thanks for doing this @eileenmcnaughton, it's been on my list to get back to this since you suggested making a default, but since we added a workaround in the feature I was working on it was lower priority. It would be great to get this fixed cleanly by adding the db default.

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 I think that UPDATE command is pretty standard mysql isn't it?

@seamuslee001
Copy link
Contributor

(CiviCRM Review Template WORD-1.1)

@seamuslee001
Copy link
Contributor

Merging

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