[IDP-1176] Add groups_external field to User
#352
Conversation
|
Fixing the bug (revealed by the tests) currently... |
PHP's `explode()` returns an array with an empty string if the given string is empty, but we do not want to add that to the `members` list.
|
|
Fixed. |
| $this->addColumn( | ||
| '{{user}}', | ||
| 'groups_external', | ||
| $this->string()->notNull()->defaultValue('')->after('groups') |
Thanks for mentioning that. The normal list of |
| 'member' => function (self $model) { | ||
| if (!empty($model->groups)) { | ||
| $member = explode(',', $model->groups); | ||
| } else { | ||
| $member = []; | ||
| } |
There was a problem hiding this comment.
Why was the else added? Is there some sort of "strict" mode that would flag an error when assigning to a new array key on a yet-undefined variable (e.g. line 581 or 585). If so, then maybe this is better:
| 'member' => function (self $model) { | |
| if (!empty($model->groups)) { | |
| $member = explode(',', $model->groups); | |
| } else { | |
| $member = []; | |
| } | |
| 'member' => function (self $model) { | |
| $member = []; | |
| if (!empty($model->groups)) { | |
| $member = explode(',', $model->groups); | |
| } |
There was a problem hiding this comment.
Using a variable without defining it (e.g. assigning something to an array on an uninitialized array, in PHP) seems like a code smell. I simply added the else to ensure that the array would exist and be in the desired state, regardless of which path through the logic was executed.
Initializing the array first, as you suggested, is another valid option, but since it would be replaced with a different array within the if-block, I decided to only set it to an empty array if needed, rather than every time.
There was a problem hiding this comment.
Thanks for discussing it, though 👍
| $this->addColumn( | ||
| '{{user}}', | ||
| 'groups_external', | ||
| $this->string()->notNull()->defaultValue('')->after('groups') |
IDP-1176 Add groups_external field to ID Broker's database
Added
groups_externalfield to UserChanged (non-breaking)
groups_externalvalues in a User'smemberlist during loginFeature PR Checklist
make composershowmake psr2Note:
This does not yet provide any mechanism for setting the new
groups_externalfield. That will come in a subsequent PR.