Fix enotice when updating a custom group with is_multiple = 1 #12243
Conversation
When updating a group with is_multiple = 1 there should be no change unless ['is_multiple'] is set but existing code is pretty confusing.
|
hey @eileenmcnaughton - yes - very simliar. the new logic makes sense to me. I did some manual testing to get my head around it. works as I would expect so good to merge. aside: not sure why we (before and now) we are entangling the need for an index update with the table name ($tableNameNeedingIndexUpdate) and not just using a boolean for $needsIndexUpdate and getting the table name from somewhere else. I would have thought that tableName should be disentangled from all those conditionals, but I am guessing you wanted to leave that part of the code alone (and I am resisting the temptation to increase the scope of your PR). |
| @@ -300,11 +300,19 @@ public function testCustomGroupCreateGroup() { | |||
| 'help_post' => 'This is Post Help For Test Group 8', | |||
| ); | |||
|
|
|||
| $customGroup = $this->callAPISuccess('custom_group', 'create', $params); | |||
| $customGroup = $this->callAPISuccess('CustomGroup', 'create', $params); | |||
There was a problem hiding this comment.
These ought to be equivalent. Any reason why this change was made?
There was a problem hiding this comment.
My understanding is that our preferred format for the entity is the camel case so it was for that reason
Overview
When updating a group with is_multiple = 1 there should be no change unless ['is_multiple']
is set but existing code is causing an e-notice in that case
Before
E-notice, new test fails
After
no e-notice, test passes
Technical Details
Comments
@michaelmcandrew perhaps you could look at this - kinda relates to yours