Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enable / Disable user from web application #27333

Merged
merged 17 commits into from
Mar 21, 2017
Merged

Enable / Disable user from web application #27333

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
5896a5a
Enable / Disable user from web application
AlexLaroche Mar 8, 2017
9bbaeea
Enable / Disable user : Remove ajax + correct UserController
AlexLaroche Mar 8, 2017
fc22600
Try to fix a test
AlexLaroche Mar 8, 2017
9438b3a
Fix typo
AlexLaroche Mar 9, 2017
da08773
Fix tests
AlexLaroche Mar 9, 2017
f0fb0e6
Fix typo
AlexLaroche Mar 9, 2017
c25d379
Minors fix
AlexLaroche Mar 9, 2017
c0c08d4
Fix bug + code improvement
AlexLaroche Mar 10, 2017
eeefa58
UI : Remove possibility to disable itself
AlexLaroche Mar 11, 2017
becc59d
UI : Permit to hide the column Enabled in users settings
AlexLaroche Mar 11, 2017
4aa1dd6
Test : Admins and sub admins cannot disabled / enabled themselves
AlexLaroche Mar 11, 2017
48c9bfa
Test : Successfully enable / disable user
AlexLaroche Mar 11, 2017
aa3590b
Test : Successfully enable / disable user
AlexLaroche Mar 11, 2017
ce4d369
Test : Successfully enable / disable user
AlexLaroche Mar 11, 2017
6d8dfae
Test : Sub admin cannot disable a user not accessible for him
AlexLaroche Mar 11, 2017
ef59718
Use OC.getCurrentUser().uid
AlexLaroche Mar 17, 2017
d31f7e1
Update settings.css
AlexLaroche Mar 20, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions settings/Controller/UsersController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ private function formatUserForIndex(IUser $user, array $userGroups = null) {
'displayname' => $user->getDisplayName(),
'groups' => (empty($userGroups)) ? $this->groupManager->getUserGroupIds($user) : $userGroups,
'subadmin' => $subAdminGroups,
'isEnabled' => $user->isEnabled(),
'quota' => $user->getQuota(),
'storageLocation' => $user->getHome(),
'lastLogin' => $user->getLastLogin() * 1000,
Expand Down Expand Up @@ -661,4 +662,68 @@ public function setDisplayName($username, $displayName) {
]);
}
}

/**
* @NoAdminRequired
*
* @param string $id
* @return DataResponse
*/
public function setEnabled($id, $enabled) {
$userId = $this->userSession->getUser()->getUID();
$user = $this->userManager->get($id);

if($userId === $id
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, prevent disabling yourself, but it seems this doesn't work correctly

&& !$this->isAdmin
&& !$this->groupManager->getSubAdmin()->isUserAccessible($this->userSession->getUser(), $user)) {
return new DataResponse(
array(
'status' => 'error',
'data' => array(
'message' => (string)$this->l10n->t('Forbidden')
)
),
Http::STATUS_FORBIDDEN
);
}


if(!$user){
return new DataResponse(
array(
'status' => 'error',
'data' => array(
'message' => (string)$this->l10n->t('Invalid user')
)
),
Http::STATUS_UNPROCESSABLE_ENTITY
);
}

if($enabled !== 'true' && $enabled !== 'false')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe convert this to a boolean directly at the beginning of the function ?
Booleans compared as strings usually give me the shivers 😄

{
return new DataResponse(
array(
'status' => 'error',
'data' => array(
'message' => (string)$this->l10n->t('Unable to enable/disable user.')
)
),
Http::STATUS_FORBIDDEN
);
}

$user->setEnabled($enabled === 'true' ? true : false);

return new DataResponse(
[
'status' => 'success',
'data' => [
'username' => $id,
'enabled' => $enabled
]
],
Http::STATUS_OK
);
}
}
47 changes: 47 additions & 0 deletions settings/js/users/users.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ var UserList = {

// initially the list might already contain user entries (not fully ajaxified yet)
// initialize these entries
this.$el.find('#isEnabled').on('change', this.onEnabledChange);
this.$el.find('.quota-user').singleSelect().on('change', this.onQuotaSelect);
},

Expand All @@ -39,6 +40,7 @@ var UserList = {
* 'displayname': 'Users display name',
* 'groups': ['group1', 'group2'],
* 'subadmin': ['group4', 'group5'],
* 'enabled' 'true'
* 'quota': '10 GB',
* 'storageLocation': '/srv/www/owncloud/data/username',
* 'lastLogin': '1418632333'
Expand Down Expand Up @@ -95,6 +97,13 @@ var UserList = {
this._updateGroupListLabel($tdSubadmins, user.subadmin);
$tdSubadmins.find('.action').tooltip({placement: 'top'});

/**
* enabled
*/
var $tdEnabled = $tr.find('#isEnabled');
$tdEnabled.attr("checked", user.isEnabled);
$tdEnabled.on('change', UserList.onEnabledChange);

/**
* remove action
*/
Expand Down Expand Up @@ -558,6 +567,44 @@ var UserList = {
);
},

/**
* Event handler for when a enabled value has been changed.
* This will save the value.
*/
onEnabledChange: function() {
var $select = $(this);
var uid = UserList.getUID($select);
var enabled = $select.attr('checked') ? 'true' : 'false';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use the boolean $select.prop('checked') for better browser compatibility

UserList._updateEnabled(uid, enabled, function(returnedEnabled){
if (enabled !== returnedEnabled) {
$select.find('#isEnabled').attr("checked", user.isEnabled);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use $select.prop('checked', user.isEnabled) for better browser compatibility

}
});
},


/**
* Saves the enabled value for the given user
* @param {String} [uid] optional user id, sets default quota if empty
* @param {String} enabled value
* @param {Function} ready callback after save
*/
_updateEnabled: function(uid, enabled, ready) {
$.post(
OC.generateUrl('/settings/users/{id}/enabled', {id: uid}),
{username: uid, enabled: enabled},
function (result) {
if(result.status == 'success') {
var msg = 'User have been ' + (result.data.enabled === 'true' ? 'enabled' : 'disabled') + '!';
OC.Notification.showTemporary(t('admin', msg));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the message directly inside the t() block instead of using a variable.
This is because the translation bot looks for the t() functions to find what text to submit for translation.

Also use a placeholder for the state: t('admin', 'User {displayName} has been {state}', {displayName: ..., state: stateText). and for the stateText: t('admin', 'enabled'), etc

} else {
OC.Notification.showTemporary(t('admin', result.data.message));
}
}
);
},


/**
* Creates a temporary jquery.multiselect selector on the given group field
*/
Expand Down
1 change: 1 addition & 0 deletions settings/routes.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
['name' => 'SecuritySettings#trustedDomains', 'url' => '/settings/admin/security/trustedDomains', 'verb' => 'POST'],
['name' => 'Users#setDisplayName', 'url' => '/settings/users/{username}/displayName', 'verb' => 'POST'],
['name' => 'Users#setMailAddress', 'url' => '/settings/users/{id}/mailAddress', 'verb' => 'PUT'],
['name' => 'Users#setEnabled', 'url' => '/settings/users/{id}/enabled', 'verb' => 'POST'],
['name' => 'Users#stats', 'url' => '/settings/users/stats', 'verb' => 'GET'],
['name' => 'LogSettings#setLogLevel', 'url' => '/settings/admin/log/level', 'verb' => 'POST'],
['name' => 'LogSettings#download', 'url' => '/settings/admin/log/download', 'verb' => 'GET'],
Expand Down
4 changes: 4 additions & 0 deletions settings/templates/users/part.userlist.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
<?php if(is_array($_['subadmins']) || $_['subadmins']): ?>
<th id="headerSubAdmins" scope="col"><?php p($l->t('Group Admin for')); ?></th>
<?php endif;?>
<th id="headerIsEnabled" scope="col"><?php p($l->t('Enabled')); ?></th>
<th id="headerQuota" scope="col"><?php p($l->t('Quota')); ?></th>
<th class="storageLocation" scope="col"><?php p($l->t('Storage Location')); ?></th>
<th class="userBackend" scope="col"><?php p($l->t('User Backend')); ?></th>
Expand Down Expand Up @@ -46,6 +47,9 @@
><span class="title groupsList"></span><span class="icon-triangle-s"></span></div>
</td>
<?php endif;?>
<td class="enabled">
<input type="checkbox" id="isEnabled" checked="checked">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use a class instead of id because ids must be unique within the page but here it would be repeated across the page. This will also require changing your jQuery selectors.

</td>
<td class="quota">
<select class="quota-user" data-inputtitle="<?php p($l->t('Please enter storage quota (ex: "512 MB" or "12 GB")')) ?>">
<option value='default'>
Expand Down
Loading