Add REST route for managing groups and users #12618

LukasReschke · 2014-12-04T13:17:12Z

First step of a somewhat testable user management. - I know, the JSON returns are in an ugly format but the JS expects it that way. So let's keep it that way until we have time to fix the JS in the future.

Todo (group):

Migrate get handler (index)
Migrate create handler (create)
Migrate destroy handler (destroy)
~~Add possibility to rename group (update)~~ (requires more work - let's do this later)
Write unit tests
Fix for subadmins

Todo (user):

Migrate get handler (index)
Migrate create handler (create)
Migrate destroy handler (destroy)
~~Add possibility to manage users (update)~~ (requires more work - let's do this later in single steps instead one large one…)
Write unit tests
Fix for subadmins

FIXME:

Fix JS code for deletion of groups

Requires #12619

@MorrisJobke

DeepDiver1975 · 2014-12-04T14:35:04Z

settings/application.php

@@ -71,6 +80,9 @@ public function __construct(array $urlParams=array()){
 		$container->registerService('L10N', function(SimpleContainer $c) {
 			return $c->query('ServerContainer')->getL10N('settings');
 		});
+		$container->registerService('GroupManager', function(SimpleContainer $c) {


I'd use IContainer - SimpleContainer is not in the public namespace

MorrisJobke · 2014-12-06T10:58:10Z

@LukasReschke Still WIP or ready for final review?

LukasReschke · 2014-12-06T12:57:08Z

@MorrisJobke Still not perfect but IMHO ready to review.

LukasReschke · 2014-12-06T13:51:11Z

Don't be shocked by that enormous amount of changes… - Most of it are unit tests…

LukasReschke · 2014-12-06T13:58:17Z

@DeepDiver1975 Care to review this as well?

@Raydiation Just FYI regarding some AppFramework usage in core. - If you want to review this as well: You're more than welcome :-) (Yes - I know that static OC_Subadmin class is annoying as hell but that's something for later ™️ )

LukasReschke · 2014-12-06T13:59:30Z

lib/private/server.php

+			$groupManager->listen('\OC\Group', 'postAddUser', function (\OC\Group\Group $group, \OC\User\User $user) {
+				\OC_Hook::emit('OC_Group', 'pre_addToGroup', array('uid' => $user->getUID(), 'gid' => $group->getGID()));
+			});
+			return $groupManager;


@icewind1991 Your feedback on that, please. THX!

MorrisJobke · 2014-12-08T10:13:19Z

settings/js/users/deleteHandler.js

@@ -188,12 +188,12 @@ DeleteHandler.prototype.deleteEntry = function(keepNotification) {

 	var payload = {};
 	payload[dh.ajaxParamID] = dh.oidToDelete;
+	console.log(dh);


I guess this needs to be removed ;)

MorrisJobke · 2014-12-08T11:05:28Z

MorrisJobke · 2014-12-08T11:08:28Z

I'm fine with this except the mentioned bug above. Once this is fixed I will give my thumbs up

First step of a somewhat testable user management. - I know, the JSON returns are in an ugly format but the JS expects it that way. So let's keep it that way until we have time to fix the JS in the future.

LukasReschke · 2014-12-08T11:11:22Z

@MorrisJobke Should be fixed now.

PVince81 · 2014-12-08T13:24:36Z

lib/private/user/user.php

@@ -153,6 +153,24 @@ public function delete() {
 			$this->emitter->emit('\OC\User', 'preDelete', array($this));
 		}
 		$result = $this->backend->deleteUser($this->uid);
+		if ($result) {
+
+			// FIXME: Feels like an hack - suggestions?


In what way ?
Should the storage removal happen somewhere else maybe, through registering to a deleteUser hook ? Maybe something for another time ?

I'd vote to have it done triggered by a hook, but this is not urgent. This goes beyond just deleting and sometimes it might (pure speculation) be useful to have the possibility to "reset" a user. But as long as we don't have the need for it, it is ok to leave it for now/OC 8 like this.

PVince81 · 2014-12-08T13:38:44Z

Code looks good apart from the HTTP codes part.

….com/owncloud/core into initial-work-migrate-to-appframework

LukasReschke · 2014-12-08T14:35:42Z

I added proper HTTP status codes with 8b3e389. - Please double check if I choose the right ones ;-)

PVince81 · 2014-12-08T15:07:19Z

BUG: Create user that already exists: no error shown in JS side. Maybe due to HTTP code change

LukasReschke · 2014-12-08T15:36:51Z

That should be fixed with c239578, though I only changed the response format and JS where necessary and not on every controller. That's something for later.

MorrisJobke · 2014-12-08T16:33:46Z

@LukasReschke Can you go to the jenkins branch and merge your changes in?

LukasReschke · 2014-12-08T16:34:46Z

Let me try…

LukasReschke · 2014-12-08T16:36:35Z

Done. - Let's wait…

scrutinizer-notifier · 2014-12-08T17:32:20Z

The inspection completed: 288 new issues, 41 updated code elements

LukasReschke · 2014-12-08T18:20:27Z

From #12693 (comment)

🚀 Test PASSed. 🚀
Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/4028/
🚀 Test PASSed. 🚀

LukasReschke · 2014-12-08T18:20:49Z

@PVince81 Can I consider this as 👍? ;-)

PVince81 · 2014-12-08T20:36:30Z

Yes, 👍

…amework Add REST route for managing groups and users

LukasReschke added the 3 - To Review label Dec 4, 2014

LukasReschke added this to the 8.0-current milestone Dec 4, 2014

LukasReschke added 2 - Developing and removed 3 - To Review labels Dec 4, 2014

LukasReschke changed the title ~~Add REST route for creating groups~~ Add REST route for managing groups Dec 4, 2014

DeepDiver1975 reviewed Dec 4, 2014
View reviewed changes

LukasReschke mentioned this pull request Dec 4, 2014

User Management - ownCloud 8.0 #12620

Closed

23 tasks

LukasReschke changed the title ~~Add REST route for managing groups~~ Add REST route for managing groups and users Dec 4, 2014

LukasReschke changed the title ~~Add REST route for managing groups and users~~ [WIP] Add REST route for managing groups and users Dec 4, 2014

LukasReschke force-pushed the initial-work-migrate-to-appframework branch from bb48151 to b15abd7 Compare December 4, 2014 18:04

LukasReschke force-pushed the initial-work-migrate-to-appframework branch from 8b84369 to acfaa24 Compare December 6, 2014 12:56

LukasReschke added 3 - To Review and removed 2 - Developing labels Dec 6, 2014

LukasReschke force-pushed the initial-work-migrate-to-appframework branch from acfaa24 to a202d01 Compare December 6, 2014 13:07

LukasReschke changed the title ~~[WIP] Add REST route for managing groups and users~~ Add REST route for managing groups and users Dec 6, 2014

LukasReschke force-pushed the initial-work-migrate-to-appframework branch from 3cc1afd to 628f917 Compare December 6, 2014 13:53

LukasReschke reviewed Dec 6, 2014
View reviewed changes

LukasReschke force-pushed the initial-work-migrate-to-appframework branch from 66cf32e to 830a3a6 Compare December 6, 2014 14:05

MorrisJobke reviewed Dec 8, 2014
View reviewed changes

Add REST route for user & group management

fe7d9a7

First step of a somewhat testable user management. - I know, the JSON returns are in an ugly format but the JS expects it that way. So let's keep it that way until we have time to fix the JS in the future.

LukasReschke force-pushed the initial-work-migrate-to-appframework branch from 227bb58 to fe7d9a7 Compare December 8, 2014 11:11

PVince81 reviewed Dec 8, 2014
View reviewed changes

MorrisJobke and others added 4 commits December 8, 2014 15:05

fix updated URL schema in JS unit tests

eac0131

Fix url

3a49411

Add statuscodes

8b3e389

Merge branch 'initial-work-migrate-to-appframework' of https://github…

5f43809

….com/owncloud/core into initial-work-migrate-to-appframework

React on other statuscodes than 200

c239578

LukasReschke added a commit that referenced this pull request Dec 8, 2014

Merge pull request #12618 from owncloud/initial-work-migrate-to-appfr…

f219f5a

…amework Add REST route for managing groups and users

LukasReschke merged commit f219f5a into master Dec 8, 2014

LukasReschke deleted the initial-work-migrate-to-appframework branch December 8, 2014 20:37

MorrisJobke mentioned this pull request Dec 22, 2014

User Management #12998

Closed

27 tasks

RobinMcCorkell mentioned this pull request Mar 9, 2015

No error message when attempting to create a group that already exists #10446

Closed

MorrisJobke mentioned this pull request Aug 7, 2015

[user mgnt] fix delete user feedback on failure #18114

Merged

lock bot locked as resolved and limited conversation to collaborators Aug 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add REST route for managing groups and users #12618

Add REST route for managing groups and users #12618

LukasReschke commented Dec 4, 2014

DeepDiver1975 Dec 4, 2014

MorrisJobke commented Dec 6, 2014

LukasReschke commented Dec 6, 2014

LukasReschke commented Dec 6, 2014

LukasReschke commented Dec 6, 2014

LukasReschke Dec 6, 2014

MorrisJobke Dec 8, 2014

MorrisJobke commented Dec 8, 2014

MorrisJobke commented Dec 8, 2014

LukasReschke commented Dec 8, 2014

PVince81 Dec 8, 2014

blizzz Dec 8, 2014

PVince81 commented Dec 8, 2014

LukasReschke commented Dec 8, 2014

PVince81 commented Dec 8, 2014

LukasReschke commented Dec 8, 2014

MorrisJobke commented Dec 8, 2014

LukasReschke commented Dec 8, 2014

LukasReschke commented Dec 8, 2014

scrutinizer-notifier commented Dec 8, 2014

LukasReschke commented Dec 8, 2014

LukasReschke commented Dec 8, 2014

PVince81 commented Dec 8, 2014

Add REST route for managing groups and users #12618

Add REST route for managing groups and users #12618

Conversation

LukasReschke commented Dec 4, 2014

DeepDiver1975 Dec 4, 2014

Choose a reason for hiding this comment

MorrisJobke commented Dec 6, 2014

LukasReschke commented Dec 6, 2014

LukasReschke commented Dec 6, 2014

LukasReschke commented Dec 6, 2014

LukasReschke Dec 6, 2014

Choose a reason for hiding this comment

MorrisJobke Dec 8, 2014

Choose a reason for hiding this comment

MorrisJobke commented Dec 8, 2014

MorrisJobke commented Dec 8, 2014

LukasReschke commented Dec 8, 2014

PVince81 Dec 8, 2014

Choose a reason for hiding this comment

blizzz Dec 8, 2014

Choose a reason for hiding this comment

PVince81 commented Dec 8, 2014

LukasReschke commented Dec 8, 2014

PVince81 commented Dec 8, 2014

LukasReschke commented Dec 8, 2014

MorrisJobke commented Dec 8, 2014

LukasReschke commented Dec 8, 2014

LukasReschke commented Dec 8, 2014

scrutinizer-notifier commented Dec 8, 2014

LukasReschke commented Dec 8, 2014

LukasReschke commented Dec 8, 2014

PVince81 commented Dec 8, 2014