Skip to content

Commit eb50704

Browse files
authored
Change Children to composite relation for modUserGroup (#15968)
* Adjust modUserGroup * phpcs * Don't use lexicons for checking admin group
1 parent 756927f commit eb50704

File tree

9 files changed

+63
-131
lines changed

9 files changed

+63
-131
lines changed

core/lexicon/en/user.inc.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@
144144
$_lang['user_group_new'] = 'Create User Group';
145145
$_lang['user_group_parent'] = 'Parent Group';
146146
$_lang['user_group_remove'] = 'Delete User Group';
147-
$_lang['user_group_remove_confirm'] = 'Are you sure you want to delete the User Group: "[[+user_group]]"?';
147+
$_lang['user_group_remove_confirm'] = 'Are you sure you want to delete the User Group: "[[+user_group]]" and all it\'s children?';
148148
$_lang['user_group_settings'] = 'User Group Settings';
149149
$_lang['user_group_settings_desc'] = 'Manage Settings for the User Group';
150150
$_lang['user_group_untitled'] = 'Untitled User Group';

core/model/schema/modx.mysql.schema.xml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1256,8 +1256,8 @@
12561256
</index>
12571257

12581258
<aggregate alias="Parent" class="MODX\Revolution\modUserGroup" local="parent" foreign="id" cardinality="one" owner="foreign" />
1259-
<aggregate alias="Children" class="MODX\Revolution\modUserGroup" local="id" foreign="parent" cardinality="many" owner="local" />
12601259
<aggregate alias="Dashboard" class="MODX\Revolution\modDashboard" local="dashboard" foreign="id" cardinality="one" owner="foreign" />
1260+
<composite alias="Children" class="MODX\Revolution\modUserGroup" local="id" foreign="parent" cardinality="many" owner="local" />
12611261
<composite alias="UserGroupMembers" class="MODX\Revolution\modUserGroupMember" local="id" foreign="user_group" cardinality="many" owner="local" />
12621262
<composite alias="FormCustomizationProfiles" class="MODX\Revolution\modFormCustomizationProfileUserGroup" local="id" foreign="usergroup" cardinality="many" owner="local" />
12631263
</object>

core/src/Revolution/Processors/Security/Group/GetNodes.php

+12-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
/*
34
* This file is part of MODX Revolution.
45
*
@@ -122,6 +123,8 @@ public function addAnonymous(array $list)
122123
$list[] = [
123124
'text' => '(' . $this->modx->lexicon('anonymous') . ')',
124125
'id' => 'n_ug_0',
126+
'expanded' => true,
127+
'allowDrop' => false,
125128
'leaf' => true,
126129
'type' => 'usergroup',
127130
'cls' => $cls,
@@ -147,15 +150,21 @@ public function prepareGroup(modUserGroup $group)
147150
$c->where(['parent' => $group->get('id')]);
148151
$c->limit(1);
149152
$count = $this->modx->getCount(modUserGroup::class, $c);
150-
return [
153+
$itemArray = [
151154
'text' => htmlentities($group->get('name'), ENT_QUOTES, 'UTF-8') . ' (' . $group->get('id') . ')',
152155
'id' => 'n_ug_' . $group->get('id'),
153-
'leaf' => $count <= 0,
156+
'hasChildren' => $count > 0,
154157
'type' => 'usergroup',
155158
'qtip' => $group->get('description'),
156159
'cls' => $cls,
160+
'allowDrop' => true,
157161
'iconCls' => 'icon icon-group',
158162
];
159-
}
160163

164+
if (!$itemArray['hasChildren']) {
165+
$itemArray['expanded'] = true;
166+
}
167+
168+
return $itemArray;
169+
}
161170
}

core/src/Revolution/Processors/Security/Group/Remove.php

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
/*
34
* This file is part of MODX Revolution.
45
*
@@ -45,6 +46,6 @@ public function beforeRemove()
4546
*/
4647
public function isAdminGroup()
4748
{
48-
return $this->object->get('id') === 1 || $this->object->get('name') === $this->modx->lexicon('administrator');
49+
return $this->object->get('id') === 1 || $this->object->get('name') === 'Administrator';
4950
}
5051
}
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
/*
34
* This file is part of MODX Revolution.
45
*
@@ -11,9 +12,7 @@
1112
namespace MODX\Revolution\Processors\Security\Group;
1213

1314
use MODX\Revolution\Processors\Processor;
14-
use MODX\Revolution\modUser;
1515
use MODX\Revolution\modUserGroup;
16-
use MODX\Revolution\modUserGroupMember;
1716
use MODX\Revolution\modX;
1817

1918
/**
@@ -56,9 +55,6 @@ public function process()
5655
}
5756

5857
$this->sortGroups($data);
59-
if ($this->modx->hasPermission('usergroup_user_edit')) {
60-
$this->sortUsers($data);
61-
}
6258

6359
return $this->success();
6460
}
@@ -75,93 +71,40 @@ public function sortGroups(array $data)
7571

7672
/* readjust groups */
7773
foreach ($groups as $groupArray) {
78-
if (!empty($groupArray['id'])) {
79-
/** @var modUserGroup $userGroup */
80-
$userGroup = $this->modx->getObject(modUserGroup::class, $groupArray['id']);
81-
if ($userGroup === null) {
82-
$this->modx->log(modX::LOG_LEVEL_ERROR,
83-
'Could not sort group ' . $groupArray['id'] . ' because it could not be found.');
84-
continue;
85-
}
86-
$oldParentId = $userGroup->get('parent');
87-
} else {
88-
$userGroup = $this->modx->newObject(modUserGroup::class);
89-
$oldParentId = 0;
74+
if (empty($groupArray['id'])) {
75+
continue;
76+
}
77+
78+
if ($groupArray['id'] === 1) {
79+
continue;
80+
}
81+
82+
/** @var modUserGroup $userGroup */
83+
$userGroup = $this->modx->getObject(modUserGroup::class, $groupArray['id']);
84+
if ($userGroup === null) {
85+
$this->modx->log(modX::LOG_LEVEL_ERROR, 'Could not sort group ' . $groupArray['id'] . ' because it could not be found.');
86+
continue;
9087
}
88+
$oldParentId = $userGroup->get('parent');
89+
9190

9291
if ($groupArray['parent'] === $userGroup->get('id')) {
9392
continue;
9493
}
9594

96-
if ($groupArray['parent'] === 0 || $oldParentId !== $groupArray['parent']) {
97-
/* get new parent, if invalid, skip, unless is root */
95+
if ($oldParentId !== $groupArray['parent']) {
9896
if ($groupArray['parent'] !== 0) {
9997
/** @var modUserGroup $parentUserGroup */
10098
$parentUserGroup = $this->modx->getObject(modUserGroup::class, $groupArray['parent']);
10199
if ($parentUserGroup === null) {
102100
continue;
103101
}
104-
$depth = $parentUserGroup->get('depth') + 1;
105-
} else {
106-
$depth = 0;
107102
}
108103

109-
/* save new parent and depth */
110104
$userGroup->set('parent', $groupArray['parent']);
111-
$userGroup->set('depth', $depth);
112-
}
113-
if ($groupArray['id'] !== 0) {
114-
$userGroup->save();
115-
}
116-
}
117-
}
118-
119-
/**
120-
* Sort and rearrange any users in the data
121-
* @param array $data
122-
* @return void
123-
*/
124-
public function sortUsers(array $data)
125-
{
126-
$users = [];
127-
$this->getUsersFormatted($users, $data);
128-
/* readjust users */
129-
foreach ($users as $userArray) {
130-
if (empty($userArray['id'])) {
131-
continue;
132-
}
133-
/** @var modUser $user */
134-
$user = $this->modx->getObject(modUser::class, $userArray['id']);
135-
if ($user === null) {
136-
continue;
137105
}
138106

139-
/* get new parent, if invalid, skip, unless is root */
140-
if ($userArray['new_group'] !== 0 && $userArray['new_group'] !== $userArray['old_group']) {
141-
/** @var modUserGroup $membership */
142-
$membership = $this->modx->getObject(modUserGroupMember::class, [
143-
'user_group' => $userArray['new_group'],
144-
'member' => $user->get('id'),
145-
]);
146-
if ($membership === null) {
147-
$membership = $this->modx->newObject(modUserGroupMember::class);
148-
$membership->set('user_group', $userArray['new_group']);
149-
}
150-
$membership->set('member', $user->get('id'));
151-
if ($membership->save()) {
152-
/* remove user from old group */
153-
if (!empty($userArray['old_group'])) {
154-
/** @var modUserGroup $oldMembership */
155-
$oldMembership = $this->modx->getObject(modUserGroupMember::class, [
156-
'user_group' => $userArray['old_group'],
157-
'member' => $user->get('id'),
158-
]);
159-
if ($oldMembership) {
160-
$oldMembership->remove();
161-
}
162-
}
163-
}
164-
}
107+
$userGroup->save();
165108
}
166109
}
167110

@@ -177,39 +120,13 @@ protected function getGroupsFormatted(&$ar_nodes, $cur_level, $parent = 0)
177120
$id = substr($id, 2); /* get rid of CSS id n_ prefix */
178121
if (substr($id, 0, 2) === 'ug') {
179122
$ar_nodes[] = [
180-
'id' => substr($id, 3),
181-
'parent' => substr($parent, 3),
123+
'id' => intval(substr($id, 3)),
124+
'parent' => intval(substr($parent, 3)),
182125
'order' => $order,
183126
];
184127
$order++;
185128
}
186129
$this->getGroupsFormatted($ar_nodes, $children, $id);
187130
}
188131
}
189-
190-
/**
191-
* @param $ar_nodes
192-
* @param $cur_level
193-
* @param int $parent
194-
*/
195-
protected function getUsersFormatted(&$ar_nodes, $cur_level, $parent = 0)
196-
{
197-
$order = 0;
198-
foreach ($cur_level as $id => $children) {
199-
$id = substr($id, 2); /* get rid of CSS id n_ prefix */
200-
if (substr($id, 0, 4) === 'user') {
201-
$userMap = substr($id, 5);
202-
$userMap = explode('_', $userMap);
203-
$ar_nodes[] = [
204-
'id' => $userMap[0],
205-
'old_group' => $userMap[1],
206-
'new_group' => substr($parent, 3),
207-
'order' => $order,
208-
];
209-
$order++;
210-
}
211-
$this->getUsersFormatted($ar_nodes, $children, $id);
212-
}
213-
}
214-
215132
}

core/src/Revolution/Processors/Security/Group/Update.php

+10
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
/*
34
* This file is part of MODX Revolution.
45
*
@@ -81,6 +82,10 @@ public function beforeSave()
8182
return $this->modx->lexicon('user_group_err_already_exists');
8283
}
8384

85+
if ($this->isAdminGroup()) {
86+
$this->object->set('parent', 0);
87+
}
88+
8489
return parent::beforeSave();
8590
}
8691

@@ -180,4 +185,9 @@ public function addUsers()
180185

181186
return $memberships;
182187
}
188+
189+
public function isAdminGroup()
190+
{
191+
return $this->object->get('id') === 1 || $this->object->get('name') === 'Administrator';
192+
}
183193
}

core/src/Revolution/mysql/modUserGroup.php

+8-8
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,14 @@ class modUserGroup extends \MODX\Revolution\modUserGroup
139139
),
140140
'composites' =>
141141
array (
142+
'Children' =>
143+
array (
144+
'class' => 'MODX\\Revolution\\modUserGroup',
145+
'local' => 'id',
146+
'foreign' => 'parent',
147+
'cardinality' => 'many',
148+
'owner' => 'local',
149+
),
142150
'UserGroupMembers' =>
143151
array (
144152
'class' => 'MODX\\Revolution\\modUserGroupMember',
@@ -166,14 +174,6 @@ class modUserGroup extends \MODX\Revolution\modUserGroup
166174
'cardinality' => 'one',
167175
'owner' => 'foreign',
168176
),
169-
'Children' =>
170-
array (
171-
'class' => 'MODX\\Revolution\\modUserGroup',
172-
'local' => 'id',
173-
'foreign' => 'parent',
174-
'cardinality' => 'many',
175-
'owner' => 'local',
176-
),
177177
'Dashboard' =>
178178
array (
179179
'class' => 'MODX\\Revolution\\modDashboard',

manager/assets/modext/widgets/security/modx.panel.user.group.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ MODx.panel.UserGroup = function(config) {
105105
,fieldLabel: _('user_group_parent')
106106
,editable: false
107107
,anchor: '100%'
108-
,disabled: config.record.id === 0
108+
,disabled: config.record.id === 0 || config.record.name === 'Administrator'
109109
,baseParams: {
110110
action: 'Security/Group/GetList'
111111
,addNone: true

manager/assets/modext/widgets/security/modx.tree.user.group.js

+8-13
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ MODx.tree.UserGroup = function(config) {
1717
,rootIconCls: 'icon-group'
1818
,root_id: 'n_ug_0'
1919
,root_name: _('user_groups')
20-
,enableDrag: true
21-
,enableDrop: true
20+
,enableDD: true
21+
,ddGroup: 'modx-usergroup-dd'
2222
,rootVisible: true
2323
,ddAppendOnly: true
2424
,useDefaultToolbar: true
@@ -181,18 +181,13 @@ Ext.extend(MODx.tree.UserGroup,MODx.tree.Tree,{
181181
}
182182

183183
,_handleDrop: function(e) {
184-
s = false;
185-
switch (e.dropNode.attributes.type) {
186-
case 'user':
187-
s = !(e.point == 'above' || e.point == 'below');
188-
s = s && e.target.attributes.type == 'usergroup' && e.point == 'append';
189-
break;
190-
case 'usergroup':
191-
s = true;
192-
break;
193-
}
194-
return s;
184+
var id = e.data.node.attributes.id.substr(2).split('_');
185+
id = parseInt(id[1]);
195186

187+
if (id === 0) return false; // block Anonymous from moving
188+
if (id === 1) return false; // block Administrator from moving
189+
190+
return true;
196191
}
197192
});
198193
Ext.reg('modx-tree-usergroup',MODx.tree.UserGroup);

0 commit comments

Comments
 (0)