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.

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

Add creating user to a service class #32857

Closed
wants to merge 2 commits into from
Closed

Conversation

sharidas
Copy link
Contributor

@sharidas sharidas commented Sep 26, 2018

Moving creation of user to a service class.

Signed-off-by: Sujith H [email protected]

Description

Add the create user to a service class. It is moved from user_management apps UsersController.

Related Issue

  • Fixes <issue_link>

Motivation and Context

Add the create user to a service class. It is moved from user_management apps UsersController. This will help us in integrating the change with the user:add command support creating users with username and email address.

How Has This Been Tested?

  • ...

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

Open tasks:

  • Backport (if applicable set "backport-request" label and remove when the backport was done)

@codecov
Copy link

codecov bot commented Sep 26, 2018

Codecov Report

Merging #32857 into master will decrease coverage by 16.17%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #32857       +/-   ##
=============================================
- Coverage     64.06%   47.88%   -16.18%     
=============================================
  Files          1190      109     -1081     
  Lines         69047    10448    -58599     
  Branches       1271     1271               
=============================================
- Hits          44234     5003    -39231     
+ Misses        24443     5075    -19368     
  Partials        370      370
Flag Coverage Δ Complexity Δ
#javascript 52.88% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 36.91% <ø> (-28.45%) 0 <ø> (-18276)
Impacted Files Coverage Δ Complexity Δ
lib/private/Files/Storage/DAV.php 59.45% <0%> (-21.64%) 0% <0%> (ø)
apps/updatenotification/templates/admin.php
lib/private/Encryption/Keys/Storage.php
lib/private/App/CodeChecker/NodeVisitor.php
lib/private/RedisFactory.php
apps/dav/lib/Avatars/AvatarNode.php
...s/dav/appinfo/Migrations/Version20170202213905.php
apps/dav/lib/Upload/ChunkLocationProvider.php
apps/files/lib/AppInfo/Application.php
apps/systemtags/list.php
... and 1066 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 70a81eb...2c8c0b5. Read the comment docs.

@sharidas sharidas force-pushed the move-createuser-toservice branch 2 times, most recently from 718bc05 to 77f025c Compare September 27, 2018 06:04
@sharidas sharidas changed the title [WIP] Add creating user to a service class Add creating user to a service class Sep 27, 2018
@sharidas sharidas assigned PVince81 and unassigned PVince81 Sep 27, 2018
@sharidas sharidas force-pushed the move-createuser-toservice branch 4 times, most recently from f9bc134 to 545c763 Compare September 27, 2018 08:37
@sharidas
Copy link
Contributor Author

Ouch.. When I run the unit test individually, I don't get failure. Will re-check again and try to fix this.

* @param IAvatarManager $avatarManager
* @param EventDispatcherInterface $eventDispatcher
*/
public function __construct(IUserSession $userSession, IGroupManager $groupManager,
Copy link
Member

Choose a reason for hiding this comment

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

Having 14 dependencies in a class is a sympton of things made wrong. Is the class really that complex, or maybe the class can be splitted in two or more?

*/
public function create($username, $password, array $groups= [], $email='') {
if ($email !== '' && !$this->mailer->validateMailAddress($email)) {
return new DataResponse(
Copy link
Member

Choose a reason for hiding this comment

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

A service implies that the code will be executed deep down the architecture, as such it shouldn't care about DataResponse nor translations because it will be handled somewhere on top (in a controller, command, or similar)

$this->appManager = $appManager;
$this->avatarManager = $avatarManager;
$this->eventDispatcher = $eventDispatcher;
$this->isAdmin = $this->isAdmin();
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather move these checks out and leave only injected objects in the constructor. Moving them out should help with the tests because other methods should use the private isAdmin() method call instead of an attribute set here.
Consider to use caching if the check is heavy, but caching should be provided by the method.

}

/**
* @param $username
Copy link
Member

Choose a reason for hiding this comment

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

missing type hint

$password = $this->secureRandom->generate(20);
}
}
$user = $this->userManager->createUser($username, $password);
Copy link
Member

Choose a reason for hiding this comment

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

If both password and email are empty, we still try to create the user with username and password. It seems wrong.

);
}

if ($user instanceof User) {
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to use if ($user !== false). In fact, I'd rather have a if ($user === false) and include the error code there. We expect the user to be created.


/**
* @param string $userId
* @param string $email
Copy link
Member

Choose a reason for hiding this comment

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

missing @throws \Exception (specially for a public method)

return $this->groupManager->isAdmin($activeUser->getUID());
}
// Check if it is triggered from command line
if (\OC::$CLI) {
Copy link
Member

Choose a reason for hiding this comment

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

Any chance to avoid this check? The service should work the same regardless of being executed by cli or not, plus this should be covered by unittests.

* @param array $userGroups
* @return array
*/
public function formatUserForIndex(IUser $user, array $userGroups = null) {
Copy link
Member

Choose a reason for hiding this comment

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

Why this method is public when it's only called inside this class? Either make it private or include a comment explaining the expected scenarios where this method will be called from other classes.

try {
$avatarAvailable = $this->avatarManager->getAvatar($user->getUID())->exists();
} catch (\Exception $e) {
//No avatar yet
Copy link
Member

Choose a reason for hiding this comment

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

Log something here, even if it's just a debug message. If something goes wrong getting the avatar, this will be impossible to detect, debug and fix. Note that you're catching any exception even the NobodyExpectsTheSpanishInquisitionException that could happen uploading such file as avatar (it's a joke, but ilustrates why it's a bad idea swallowing exceptions like this)

@jvillafanez
Copy link
Member

I've just checked the SigninWithEmail class, not everything. I expect some changes that might affect the rest of the code.

* use one time. The new user has to reset it using the link
* from email.
*/
$event = new GenericEvent();
Copy link
Member

Choose a reason for hiding this comment

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

Any chance of using a custom event instead of a generic one? A custom event would be better because it can have a specific method to set the password we want to use, as well as document what exceptions are allowed to be thown from the event handler, if any.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a new custom event. Let me know if this looks ok. As part of this change I made modification to password_policy app. Hence owncloud/password_policy#154, needs review too.

throw new CannotCreateUserException($message);
}

if ($user instanceof User) {
Copy link
Member

Choose a reason for hiding this comment

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

It might be better to reverse the logic:

if ($user === null) {
  throw new CannotCreateUserException(....);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated as per the suggestion.

return $this->groupManager->isAdmin($activeUser->getUID());
}
// Check if it is triggered from command line
if (\OC::$CLI) {
Copy link
Member

Choose a reason for hiding this comment

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

any chance not to depend on this static variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the dependency with the static variable.

* use one time. The new user has to reset it using the link
* from email.
*/
$passwordCreateEvent = new UserPasswordCreateEvent(UserPasswordCreateEvent::CREATE_PASSWORD, []);
Copy link
Member

Choose a reason for hiding this comment

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

We have to keep backwards compatibility. I don't think we have a migration path for these events yet, so it's better not to touch it.
My point is that we can't assume that the app will be updated, or even use the new event. The apps need to work anyway. If the app is still using the old GenericEvent it will likely crash.

The idea is fine, but the old event still needs to be thrown. Maybe @PVince81 can decide how and when we want to move this forward.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have reverted the code and brought back the GenericEvent.

return $this->groupManager->isAdmin($activeUser->getUID());
}
// Check if it is triggered from command line
$cli = \in_array(\php_sapi_name(), ['cli']);
Copy link
Member

Choose a reason for hiding this comment

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

The point of touching this was to make unittests easier. Even though the tests should mainly test public methods, all the code in the class should be covered by those unittests. Right now, this isn't possible because the php_sapi_name can't be mocked, which means that this method will never return false, and the corresponding code path won't be covered by unittests.

If you can't find a better solution, use the previous code (using the OC::$CLI) because we can change it inside the tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have moved php_sapi_name to a method so that it can be mocked.

@sharidasan sharidasan force-pushed the move-createuser-toservice branch 3 times, most recently from d61b730 to 1b65bab Compare October 24, 2018 07:34
@@ -156,13 +164,27 @@ public function addUser() {
}

try {
$newUser = $this->userManager->createUser($userId, $password);
$this->logger->info('Successful addUser call with userid: '.$userId, ['app' => 'ocs_api']);
$password = ($password === null) ? '' : $password;
Copy link
Member

Choose a reason for hiding this comment

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

this can be done above, as part of the initialization (where all the isset($_POST[...]) are)

Choose a reason for hiding this comment

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

Done.

* @param string $username
* @param string $password can be empty string, else string for password
* @param string $email
* @return bool|\OCP\IUser when user is created else false
Copy link
Member

Choose a reason for hiding this comment

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

I still don't know when the boolean value is returned. You're either returning the created user object or throwing an exception. If the created user object is false, you're also throwing an exception.

Choose a reason for hiding this comment

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

Updated the doc.

$userAddedToGroups = ($groups === null) ? [null] : $groups;
$user = $this->createUserService->createUser($userId, $password, $emailAddress);
if ($user instanceof User) {
$newUser = $this->userManager->get($userId);
Copy link
Member

Choose a reason for hiding this comment

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

This seems pointless. You should already have the instance from the createUser call.

Choose a reason for hiding this comment

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

Removed.

}

/**
* Add user to the groups
Copy link
Member

Choose a reason for hiding this comment

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

Just reading this I assume that, if for whatever reason the user can't be added to a group (It's an LDAP group, or there is a problem saving the data), the function will return false. This isn't true because the function will return true as long as the user is added to just one group.
There is also no clarification about what happens if the user isn't added to a group: should the code ignore it? should remove the assignment to previous groups and abort?

Clarify the expected behaviour and fix whatever is needed.

Choose a reason for hiding this comment

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

Updated the doc. Let me know if this sounds reasonable.


if (\is_array($groups)) {
$result = $this->createUserService->addUserToGroups($user, $userAddedToGroups);
if ($result === false) {
$this->logger->error("User $userId could not be added to groups");
Copy link
Member

Choose a reason for hiding this comment

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

It might be nice to know what are those groups

Choose a reason for hiding this comment

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

With the latest change, the user should be able to know about the groups which are not added.

}
} else {
$output->writeln("<error>Interactive input or --password-from-env is needed for entering a password!</error>");
return 1;
Copy link
Member

Choose a reason for hiding this comment

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

Try to return different values for each error type. This will help to identify what happened if these commands are used in a script.

Choose a reason for hiding this comment

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

Returning different values.

core/routes.php Outdated
['name' => 'user#setPassword', 'url' => '/setpassword/{token}/{userId}', 'verb' => 'POST'],
['name' => 'user#resendToken', 'url' => '/resend/token/{userId}', 'verb' => 'POST'],
['name' => 'user#setPasswordForm', 'url' => '/setpassword/form/{token}/{userId}', 'verb' => 'GET'],
['name' => 'avatar#getAvatar', 'url' => '/avatar/{userId}/{size}', 'verb' => 'GET'],
Copy link
Member

Choose a reason for hiding this comment

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

where this come from?

Choose a reason for hiding this comment

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

Removed.

public function addUserToGroups(IUser $user, array $groups= []) {
$currentUser = $this->userSession->getUser();
if (!$this->isAdmin($currentUser)) {
if (!empty($groups)) {
Copy link
Member

Choose a reason for hiding this comment

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

You might want to add a comment to explain what you're doing here, or extract this piece to another function. The code seems big, and I'm pretty sure you aren't add users to groups here.
It's fine to have some setup or preparation, but if it's too big it might be confusing.

Choose a reason for hiding this comment

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

Moved to a different method.

if (!empty($groups)) {
foreach ($groups as $key => $group) {
$groupObject = $this->groupManager->get($group);
if ($groupObject === null) {
Copy link
Member

Choose a reason for hiding this comment

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

if ($groupObject === null || !$this->isSubadminOfGroup($currentUser, $groupObject))

In fact, you can move the check for null inside the isSubadminOfGroup function to simplify this piece.

return $this->groupManager->isAdmin($currentUser->getUID());
}
// Check if it is triggered from command line
$cli = \OC::$CLI;
Copy link
Member

Choose a reason for hiding this comment

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

just return \OC::$CLI. It's more straightforward

Choose a reason for hiding this comment

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

Done.

@sharidasan sharidasan force-pushed the move-createuser-toservice branch 3 times, most recently from aded134 to a1ce6f6 Compare October 24, 2018 15:42
$user = $this->userSession->getUser();
$isAdmin = $this->groupManager->isAdmin($user->getUID());
$subAdminManager = $this->groupManager->getSubAdmin();
$password = ($password === null) ? '' : $password;
Copy link
Member

Choose a reason for hiding this comment

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

move it upper, line 135. Initialize the same way you're doing with the emailAddress.

@@ -156,13 +165,28 @@ public function addUser() {
}

try {
$newUser = $this->userManager->createUser($userId, $password);
$this->logger->info('Successful addUser call with userid: '.$userId, ['app' => 'ocs_api']);
$groups = ($groups === null) ? [null] : $groups;
Copy link
Member

Choose a reason for hiding this comment

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

is this needed? any more clear way to do it?

$this->logger->info('Successful addUser call with userid: '.$userId, ['app' => 'ocs_api']);
$groups = ($groups === null) ? [null] : $groups;
$newUser = $this->createUserService->createUser($userId, $password, $emailAddress);
if ($newUser instanceof User) {
Copy link
Member

Choose a reason for hiding this comment

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

This check doesn't seem to be needed.

*
* @param IUser $user
* @param array $groups contains array of group names
* @return array with bool, true if user is added to groups, else false, and array of groups failed to add user
Copy link
Member

Choose a reason for hiding this comment

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

It's better to return just the list of groups that weren't added. We can assume that if the returned list is empty, the user has been added to all the groups correctly.

if ($result === false) {
$failedGroups = \implode(',', $failedGroups);
$this->logger->error("User $userId could not be added to groups " . $failedGroups);
}
foreach ($groups as $group) {
Copy link
Member

Choose a reason for hiding this comment

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

this seems to be duplicated. The user should have been added to the groups with the addUserToGroups

return 7;
} else {
foreach ($groupInput as $groupName) {
if ($this->groupManager->isInGroup($uid, $groupName)) {
Copy link
Member

Choose a reason for hiding this comment

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

Knowing the groups that have failed, you should know what are the ones that succeeded, no need to ask the groupManager.

Choose a reason for hiding this comment

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

updated.

* @param array $groups contains group names
* @return array
*/
private function setUpGroupsForUser(IUser $currentUser, $groups) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think the name reflects what the function does. I don't get what this function is for.... assuming that the currentUser is a subadmin, make sure the $groups fall into the groups controlled by the subadmin? If this is it, we might need to have a look at this because this seems quite complex for that.

*
* @param IUser $user
* @param array $groups contains array of group names
* @return array with bool, true if user is added to groups or if no groups are there, else false, and array of groups failed to add user
Copy link
Member

Choose a reason for hiding this comment

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

if feels better if you just return the list of failed groups. If the list is empty, the user has been added to all the groups, so no need to return the boolean.

$userAddedToGroup = false;
if (\count($groups) >= 1) {
foreach ($groups as $groupName) {
if ($groupName !== null) {
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to check for null here? I expect the list not to contain any null, otherwise we'll likely be doing something wrong somewhere.

Choose a reason for hiding this comment

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

if (empty($groupObject)) {
$groupObject = $this->groupManager->createGroup($groupName);
}
$groupObject->addUser($user);
Copy link
Member

Choose a reason for hiding this comment

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

I think the addUser doesn't return anything, so we don't know whether the user has been effectively added to the group or not (it doesn't throw any "known" exception as far as I know). It might be interesting to add a parameter to indicate whether we should explicitly check here if the user has been added or not.

@CLAassistant
Copy link

CLAassistant commented Oct 25, 2018

CLA assistant check
All committers have signed the CLA.

@sharidasan sharidasan force-pushed the move-createuser-toservice branch 5 times, most recently from ca3a5bd to 4791191 Compare October 25, 2018 17:08
Moving creation of user to a service class.

Signed-off-by: Sujith H <[email protected]>
@@ -126,7 +133,9 @@ public function getUsers() {
public function addUser() {
$userId = isset($_POST['userid']) ? $_POST['userid'] : null;
$password = isset($_POST['password']) ? $_POST['password'] : null;
$password = ($password === null) ? '' : $password;
Copy link
Member

Choose a reason for hiding this comment

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

This isn't needed.

$password = isset($_POST['password']) ? $_POST['password'] : ''

Copy link

@sharidasan sharidasan Oct 26, 2018

Choose a reason for hiding this comment

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

Updated

@@ -156,13 +165,15 @@ public function addUser() {
}

try {
$newUser = $this->userManager->createUser($userId, $password);
$groups = ($groups === null) ? [] : $groups;
Copy link
Member

Choose a reason for hiding this comment

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

Move this above, as part of the initiailization. Also do the same as with the password. Also make sure the variable is always an array.

Choose a reason for hiding this comment

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

Done.

@@ -156,13 +165,15 @@ public function addUser() {
}

try {
$newUser = $this->userManager->createUser($userId, $password);
$groups = ($groups === null) ? [] : $groups;
$newUser = $this->createUserService->createUser($userId, $password, $emailAddress);
$this->logger->info('Successful addUser call with userid: '.$userId, ['app' => 'ocs_api']);

if (\is_array($groups)) {
Copy link
Member

Choose a reason for hiding this comment

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

we should try to remove this check by making sure this will always be an array.

Choose a reason for hiding this comment

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

Removed the check. And one round of manual testing it was working. Will re-check again.

$failedGroups = $this->createUserService->addUserToGroups($newUser, $groupInput);
if (\count($failedGroups) > 0) {
$failedGroups = \implode(',', $failedGroups);
$output->writeln('<warning>Unable to add user: ' . $uid . ' to groups ' . $failedGroups . '.</warning>');
Copy link
Member

Choose a reason for hiding this comment

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

try to use string interpolation as much as possible.

Choose a reason for hiding this comment

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

Mostly done. Need to re-check again.

return 7;
} else {
foreach ($groupInput as $groupName) {
if ($this->groupManager->isInGroup($uid, $groupName)) {
Copy link
Member

Choose a reason for hiding this comment

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

This check is already done in the addUserToGroups, no need to duplicate.

Choose a reason for hiding this comment

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

Removed.

// init some group shares.
Filesystem::init($user->getUID(), '');
}
$newUser = $this->createUserService->createUser($uid, $password, $email);
Copy link
Member

Choose a reason for hiding this comment

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

double-check if there is any difference with the code added above. Move the code to a private function if possible to avoid the duplication.

Choose a reason for hiding this comment

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

I will update this change.

public function addUserToGroups(IUser $user, array $groups= [], $checkInGroup = true) {
$failedToAdd = [];

if (\count($groups) >= 1) {
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be needed. The loop won't be executed if the array is empty.

Choose a reason for hiding this comment

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

Removed


if (\count($groups) >= 1) {
foreach ($groups as $groupName) {
if ($groupName !== '') {
Copy link
Member

Choose a reason for hiding this comment

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

any reason for this check?

Choose a reason for hiding this comment

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

Removed

if ($checkInGroup && !$this->groupManager->isInGroup($user->getUID(), $groupName)) {
$failedToAdd[] = $groupName;
} else {
$this->logger->info('Added userid ' . $user->getUID() . ' to group ' . $groupObject->getGID());
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to remove this log because it might be inaccurate, at least with the current code. If the checkInGroup is false, the user might be be added but we'll show a log stating that the user has been added. This will be very troublesome to debug.

Choose a reason for hiding this comment

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

Removed.

Update the provisioning api for new user registration with
email.

Signed-off-by: Sujith H <[email protected]>
@DeepDiver1975
Copy link
Member

outdated -> close

@DeepDiver1975 DeepDiver1975 deleted the move-createuser-toservice branch July 19, 2019 09:08
@sharidas sharidas restored the move-createuser-toservice branch July 19, 2019 11:40
@sharidas
Copy link
Contributor Author

Reopening this PR again.

@sharidas sharidas reopened this Jul 19, 2019
@micbar
Copy link
Contributor

micbar commented Jul 19, 2019

@sharidas This PR seems very outdated.
We do not need UI parts, they are already done.

Wouldn't it be better to start from scratch?

@DeepDiver1975
Copy link
Member

As discussed in #35777 the master branch will from now on hold the ownCloud 10 codebase.

This PR targetted ownCloud 11 which is postponed to a far distant future.

Because of that I'm closing this PR and kindly ask you to re-submit this PR in a few days.

Thanks a lot for your patience

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants