Fix problem with group.update api #26260
Conversation
|
(Standard links)
|
samuelsov
changed the title
samuelsov
changed the title
|
@samuelsov probably just delete line 339? |
It is converted to an array a bit later in the code so I moved the check to the very beginning of the method. |
| @@ -384,6 +388,10 @@ public static function create(&$params) { | |||
|
|
|||
| if (!CRM_Utils_System::isNull($params['parents'])) { | |||
| $params['parents'] = CRM_Utils_Array::convertCheckboxFormatToArray((array) $params['parents']); | |||
There was a problem hiding this comment.
Refactoring note: not a blocker for this PR, but this function really shouldn't be called from the BAO as it's only needed on the quickform.
|
@samuelsov are you able to rebase this into a single commit? |
|
@colemanw ok, yeah, done (I think). I though you could do it when merging like in gitlab. |
|
That would be nice! But our workflow currently requires a merge-commit. @samuelsov I think the only thing missing here is a unit test. Are you up for that? |
|
@colemanw yes, hopefully tomorrow :) |
|
That would be great thanks @samuelsov |
|
@colemanw seems good to me, the tests detected an error that I fixed. Can you review ? |
| @@ -448,7 +456,7 @@ public static function create(&$params) { | |||
| } | |||
There was a problem hiding this comment.
This block (see above) will actually force in a parents param, so we either need to alter $parentsParamProvided here or else modify the check below for refreshing the cache.
There was a problem hiding this comment.
Ok, any preference ? we could also go back to a safer approach where the cache is updated no matter what.
CRM/Contact/BAO/Group.php
Outdated
| // parent group is removed | ||
| CRM_Contact_BAO_GroupNestingCache::update(); | ||
| // refresh cache if parents param was provided | ||
| if ($parentsParamProvided) { |
There was a problem hiding this comment.
How about this?
| if ($parentsParamProvided) { | |
| if ($parentsParamProvided || !empty($params['parents'])) { |
There was a problem hiding this comment.
Ok done. It may be useful to add some unit tests to check the group nesting cache although not sure I'm the good person for that.
colemanw
changed the title
Overview
Following PR #26200, there are 3 things to fix/improve in
CRM_Contact_BAO_Group::update:CRM_Contact_BAO_GroupNesting::getParentGroupIds