dev/core/287 Disable child groups if all parents are disabled #12797
Conversation
|
Can one of the admins verify this patch? |
CRM/Contact/BAO/Group.php
Outdated
| @@ -350,6 +350,23 @@ public static function create(&$params) { | |||
| CRM_Utils_Hook::pre('create', 'Group', NULL, $params); | |||
| } | |||
|
|
|||
| // CRM-287 Disable child groups if all parents are disabled. | |||
There was a problem hiding this comment.
CRM-287? This is not the correct issue number!
There was a problem hiding this comment.
I think this whole process should be done after parent group is disabled.
There was a problem hiding this comment.
Updated the issue number.
There was a problem hiding this comment.
And about the workflow. Lets consider a situation with three parent groups to one child group. The child group will be disabled after the 3rd parent is disabled. But when any of the parent is enabled the child should be enabled too. So I think the current workflow is right.
There was a problem hiding this comment.
@madhavimalgaonkar Can you iron out more of my confusion for below use case(or isn't a valid use case)?
- Disabled child group.
- After some days Disabled
#1'sParent group. - Later Enabled
#1'sParent group, but don't want the child group to be enabled.
| } | ||
| if (count($parentIds) > 1 && $activeParentsCount <= 1) { | ||
| $setDisable = self::setIsActive($childValue, $params['is_active']); | ||
| } |
There was a problem hiding this comment.
Can we group 362 and 365 with OR statement?
There was a problem hiding this comment.
Thanks for the point :) . Have updated the conditions.
CRM/Contact/BAO/Group.php
Outdated
| @@ -350,6 +350,20 @@ public static function create(&$params) { | |||
| CRM_Utils_Hook::pre('create', 'Group', NULL, $params); | |||
| } | |||
|
|
|||
| // dev/core#287 Disable child groups if all parents are disabled. | |||
| if(!empty($params['id'])) { | |||
There was a problem hiding this comment.
should be if (!empty($params['id'])) {
CRM/Contact/BAO/Group.php
Outdated
| 'id' => ['IN' => $parentIds], | ||
| 'is_active' => 1, | ||
| ]); | ||
| if (count($parentIds) == 1 || count($parentIds) > 1 && $activeParentsCount <= 1) { |
There was a problem hiding this comment.
can this be written as if (count($parentIds) >= 1 && $activeParentsCount <= 1) { ?
|
test this please |
|
@lcdservices @pradpnayak is this mergeable? |
|
I ran through a simple test and it appeared to work as expected:
|
|
merging based on @lcdservices review & @pradpnayak code input. Thank you @lcdservices @pradpnayak and @madhavimalgaonkar |
|
This looks like to be causing 2 failures @eileenmcnaughton as per https://test.civicrm.org/job/CiviCRM-Core-Matrix/BKPROF=min,CIVIVER=master,label=bknix-tmp/5811/ |
Fix unit tests following merge of PR #12797
Overview
Child groups with all parents disabled shows in group list
Before
How it works currently: If a child group has multiple parent groups and one of them is disabled, the child group should to show up on group selector lists. This has been fixed here https://issues.civicrm.org/jira/browse/CRM-20934.
If all parent group(s) are disabled the child group still shows up in search mode.
You can see the child group with title as Child group Child of: With null value for parent.
image
After
If a child group has multiple or single parent group and all of them are disabled, then the child group be
disabled and removed from the selector lists.
Technical Details
Added warning before disabling any group. Checked for groups with both single and multiple parent groups.
https://lab.civicrm.org/dev/core/issues/287