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

[WIP] Move user creation to a separate service #35917

Closed
wants to merge 1 commit into from

Conversation

sharidas
Copy link
Contributor

@sharidas sharidas commented Jul 24, 2019

Move user creation to a separate service.

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

Description

Move the user creation to the service class.

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. It will also help for provisioning API.

How Has This Been Tested?

  • Verified that from webUI users can be created using users page, with and without email address
  • Verified that from webUI users can be created using users page and while creation the user could be added to the group. Tested with and without email address.
  • Verified in the settings page of the logged in user and in the users page of admin that the group associated with the user is reflected.
  • Verified that using user:add command the users are created with email address and/or with password. For example:
sujith@sujith-ownCloud  ~/test/owncloud   move-createuser-toservice-v2 ●  ./occ user:add --display-name "Test User5" --email "***********" --group "group1" --group "group2" user5
Cannot load Xdebug - it was already loaded
The user "user5" was created successfully
Display name set to "Test User5"
Email address set to "************"
User user5 added to group group1
User user5 added to group group2
 sujith@sujith-ownCloud  ~/test/owncloud   move-createuser-toservice-v2 ●  
  • In the above example group1 and group2 were created and the user5 is added to the group
  • An email is sent to to the user to set the password, if the email is configured in the sever.
  • Verified that user can set the pasword.
  • Verified using provisioningapi the users are created with and without email address, for example:
sujith@sujith-ownCloud  ~/test/owncloud   move-createuser-toservice-v2 ●  curl -X POST http://admin:admin@localhost/testing/ocs/v1.php/cloud/users -d userid="test6" -d email='********' -d groups\[\]="group1" -d groups\[\]="group2"                           
<?xml version="1.0"?>
<ocs>
 <meta>
  <status>ok</status>
  <statuscode>100</statuscode>
  <message/>
 </meta>
 <data/>
</ocs>
 sujith@sujith-ownCloud  ~/test/owncloud   move-createuser-toservice-v2 ● 
  • From the above example verified that password could be reset from the email received.
  • Another example using password:
 sujith@sujith-ownCloud  ~/test/owncloud   move-createuser-toservice-v2 ●  curl -X POST http://admin:admin@localhost/testing/ocs/v1.php/cloud/users -d userid="test7" -d password="1" -d email='*********' -d groups\[\]="group2" -d groups\[\]="group1"
<?xml version="1.0"?>
<ocs>
 <meta>
  <status>ok</status>
  <statuscode>100</statuscode>
  <message/>
 </meta>
 <data/>
</ocs>
 sujith@sujith-ownCloud  ~/test/owncloud   move-createuser-toservice-v2 ● 
  • Was able to login as test7 user with the password provided in the above example. Also verified that email address is added to the user.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

Open tasks:

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

@codecov
Copy link

codecov bot commented Jul 25, 2019

Codecov Report

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

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #35917       +/-   ##
=============================================
- Coverage      65.8%   49.03%   -16.77%     
=============================================
  Files          1229      109     -1120     
  Lines         70856    10535    -60321     
  Branches       1289     1289               
=============================================
- Hits          46626     5166    -41460     
+ Misses        23852     4991    -18861     
  Partials        378      378
Flag Coverage Δ Complexity Δ
#javascript 53.7% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 38.59% <ø> (-28.6%) 0 <ø> (-18760)
Impacted Files Coverage Δ Complexity Δ
lib/private/Files/Storage/DAV.php 58.71% <0%> (-21.37%) 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 1105 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 e208e43...e51312f. Read the comment docs.

@codecov
Copy link

codecov bot commented Jul 25, 2019

Codecov Report

Merging #35917 into master will decrease coverage by 0.03%.
The diff coverage is 52.25%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #35917      +/-   ##
============================================
- Coverage      65.8%   65.76%   -0.04%     
- Complexity    18760    18817      +57     
============================================
  Files          1229     1241      +12     
  Lines         70856    71140     +284     
  Branches       1289     1289              
============================================
+ Hits          46626    46786     +160     
- Misses        23852    23976     +124     
  Partials        378      378
Flag Coverage Δ Complexity Δ
#javascript 53.7% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 67.14% <52.25%> (-0.05%) 18817 <54> (+57)
Impacted Files Coverage Δ Complexity Δ
core/routes.php 65.51% <ø> (ø) 0 <0> (ø) ⬇️
core/templates/new_user/resendtokenbymail.php 0% <0%> (ø) 0 <0> (?)
core/Application.php 38.05% <0%> (-1.18%) 25 <0> (ø)
...lic/User/Exceptions/UserTokenMismatchException.php 0% <0%> (ø) 1 <1> (?)
core/templates/new_user/email-html.php 0% <0%> (ø) 0 <0> (?)
core/templates/new_user/setpassword.php 0% <0%> (ø) 0 <0> (?)
core/templates/new_user/email-plain_text.php 0% <0%> (ø) 0 <0> (?)
core/templates/new_user/tokensendnotify.php 0% <0%> (ø) 0 <0> (?)
core/register_command.php 0% <0%> (ø) 0 <0> (ø) ⬇️
...blic/User/Exceptions/InvalidUserTokenException.php 100% <100%> (ø) 1 <1> (?)
... and 22 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 bd34727...8571ea2. Read the comment docs.

@sharidas sharidas force-pushed the move-createuser-toservice-v2 branch 2 times, most recently from 9d68c79 to c4aeb19 Compare July 26, 2019 09:18
@sharidas sharidas requested a review from jvillafanez July 26, 2019 11:11
lib/private/User/Service/CreateUserService.php Outdated Show resolved Hide resolved
lib/private/User/Service/CreateUserService.php Outdated Show resolved Hide resolved
lib/private/User/Service/CreateUserService.php Outdated Show resolved Hide resolved
lib/private/User/Service/CreateUserService.php Outdated Show resolved Hide resolved
core/Command/User/Add.php Outdated Show resolved Hide resolved
@sharidas sharidas force-pushed the move-createuser-toservice-v2 branch from c4aeb19 to 1654a57 Compare July 26, 2019 16:21
Move user creation to a separate service.

Signed-off-by: Sujith H <[email protected]>
@sharidas sharidas force-pushed the move-createuser-toservice-v2 branch from 1654a57 to 8571ea2 Compare July 29, 2019 08:11
@jvillafanez
Copy link
Member

Is it everything covered with unittests? Any piece that can't be easily covered?

@DeepDiver1975 DeepDiver1975 changed the base branch from master to archive/master July 30, 2019 08:30
@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.

3 participants