Fix display name ignored when creating new user #9242

danxuliu · 2018-04-18T22:41:25Z

Follow-up to #7600

Before #7600 when a new user was created it was possible to set its user name, password and groups. In #7600 the display name and the e-mail were added to the form, but the display name was not taken into account when the new user was created (the e-mail, on the other hand, was).

This pull request adds an optional parameter to the create user endpoint to make possible to specify the display name and sends the display name set (if any) in the form when creating a user.

How to test

Open the Users settings
Create a new user with an explicit display name

Expected result
The display name of the new user is set to the desired one.

Actual result
The display name of the new user is the default display name, which is the same as the user name.

codecov · 2018-04-18T23:08:13Z

Codecov Report

Merging #9242 into master will increase coverage by 0.66%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #9242      +/-   ##
============================================
+ Coverage     51.04%   51.71%   +0.66%     
+ Complexity    26797    25672    -1125     
============================================
  Files          1708     1634      -74     
  Lines         99644    95815    -3829     
  Branches       1327     1382      +55     
============================================
- Hits          50864    49551    -1313     
+ Misses        48780    46264    -2516

Impacted Files	Coverage Δ	Complexity Δ
core/Command/Maintenance/Mode.php	`0% <0%> (-83.79%)`	`6% <0%> (-2%)`
apps/dav/lib/CalDAV/CalendarRoot.php	`0% <0%> (-75%)`	`1% <0%> (-3%)`
core/js/public/comments.js	`68.18% <0%> (-22.73%)`	`0% <0%> (ø)`
lib/private/Mail/Mailer.php	`61.36% <0%> (-13.06%)`	`28% <0%> (+1%)`
apps/oauth2/lib/Settings/Admin.php	`92.3% <0%> (-7.7%)`	`4% <0%> (+1%)`
apps/sharebymail/lib/ShareByMailProvider.php	`58.8% <0%> (-7.63%)`	`85% <0%> (-10%)`
apps/theming/lib/ThemingDefaults.php	`89.79% <0%> (-7.51%)`	`51% <0%> (-1%)`
lib/private/Authentication/Token/DefaultToken.php	`85.24% <0%> (-6.69%)`	`20% <0%> (ø)`
apps/oauth2/lib/Controller/SettingsController.php	`72.41% <0%> (-5.59%)`	`3% <0%> (-3%)`
lib/private/L10N/Factory.php	`60.82% <0%> (-5.43%)`	`88% <0%> (-23%)`
... and 324 more

MorrisJobke · 2018-04-19T00:17:01Z

That explains why I was confused because the displays me was not how I thought I typed it in😅

skjnldsv · 2018-04-19T06:14:11Z

Since we're going to switch to the new users list, all of this is fixed already since we're now using our ocs api to create users. Old legacy code like this one is being cleaned :/

MorrisJobke

Tested and works 👍

danxuliu · 2018-04-20T13:56:22Z

I have amended the last commit as I had forgotten to add the step to the matrix (and thus it was not executed by Drone).

MorrisJobke · 2018-04-20T15:15:17Z

CI failure is unrelated.

skjnldsv · 2018-04-20T15:18:12Z

Did someone read my message above? 😁 ❤️

danxuliu · 2018-04-23T08:55:49Z

@skjnldsv

Since we're going to switch to the new users list, all of this is fixed already since we're now using our ocs api to create users. Old legacy code like this one is being cleaned :/

All of this is fixed already... but where? :-P I mean, in current master if you create a user with a custom display name that name is ignored, so I guess that it is fixed in some branch that will become a pull request in the future. Therefore, if these changes affect or conflict in any way with your work you can close this, but if they do not I would merge this now even if it is going to be thrown away in the future.

skjnldsv · 2018-04-23T09:53:54Z

@danxuliu aaand you're right, somehow I missed that!
I thought I pushed it already :/

server/apps/provisioning_api/lib/Controller/UsersController.php

Lines 205 to 211 in 3ff041f

    
           public function addUser(string $userid, 
        
           						string $password = '', 
        
           						string $email = '', 
        
           						array $groups = [], 
        
           						array $subadmin = [], 
        
           						string $quota = '', 
        
           						string $language = ''): DataResponse {

MorrisJobke · 2018-05-07T13:47:43Z

@skjnldsv @danxuliu What is the status here?

skjnldsv · 2018-05-07T13:52:10Z

My point (tell me what you think @danxuliu :) ) is: since we want to drop old legacy stuf (so the Userscontroller users management parts), maybe we should edit this pr to add the new method to the ocs api instead :)

This way we can easily add this ti the vue settings pr as well ;)

danxuliu · 2018-05-07T15:59:19Z

@skjnldsv

since we want to drop old legacy stuf (so the Userscontroller users management parts), maybe we should edit this pr to add the new method to the ocs api instead :)

But then you would be using the OCS API to create new users while the rest of settings/js/users/users.js would be using the legacy AJAX calls. The new and the legacy parts can work together without problems, right?

Personally I would merge the fix now and then add another pull request to move to the new API instead of fixing while moving, but it is just my usual nitpicking, so you have my blessing if you want to do it that way too ;-)

skjnldsv · 2018-05-07T16:03:00Z

@danxuliu could you add the input to the ocs api anyway?
It will be a pain to rebase though :p

As a record, this will be override completely this week with the new migration ;)

danxuliu · 2018-05-07T16:14:09Z

could you add the input to the ocs api anyway?

Sure ;-)

It will be a pain to rebase though :p

Rebase what onto what?

this will be override completely this week with the new migration ;)

You do not need to subtly try to change my mind, I already told you that you can do it the other way if you want :-P

MorrisJobke · 2018-05-23T18:26:31Z

Rebased on master. Basically the code was dropped but then we have at least the acceptance tests 😉

skjnldsv · 2018-05-23T18:33:13Z

We still need to add this to the ocs api

MorrisJobke · 2018-07-04T11:50:15Z

@skjnldsv Status of this?

skjnldsv · 2018-07-04T12:31:39Z

@MorrisJobke ocs api is still not included in this pr: @danxuliu

server/apps/provisioning_api/lib/Controller/UsersController.php

Lines 210 to 216 in 3ffeb4a

    
           public function addUser(string $userid, 
        
           						string $password = '', 
        
           						string $email = '', 
        
           						array $groups = [], 
        
           						array $subadmin = [], 
        
           						string $quota = '', 
        
           						string $language = ''): DataResponse {

danxuliu · 2018-07-24T11:38:32Z

ocs api is still not included in this pr

I will take care of that, although I want to fix some things in the acceptance tests before doing it (of course if that takes me longer than expected I will send these changes anyway before Beta 1 2).

Signed-off-by: Daniel Calviño Sánchez <[email protected]>

Although the form to create a new user included a field to set the display name its value was not taken into account, so the new user ended with the default display name (the same as the user name). Signed-off-by: Daniel Calviño Sánchez <[email protected]>

Signed-off-by: Daniel Calviño Sánchez <[email protected]>

danxuliu · 2018-07-31T11:46:05Z

ocs api is still not included in this pr

I will take care of that, although I want to fix some things in the acceptance tests before doing it

I will not be able to fix those things in the acceptance tests before Beta 2, so I have updated this pull request with the fix to the OCS API. This is now ready to be reviewed.

Signed-off-by: Roeland Jago Douma <[email protected]>

rullzer · 2018-07-31T13:04:41Z

I pushed the compiled vue stuff

danxuliu · 2018-07-31T17:31:39Z

@rullzer

I pushed the compiled vue stuff

I had built (running just make in the settings dir) and pushed the built files along with the commit in which I modified the JavaScript, so I do not know why https://drone.nextcloud.com/nextcloud/server/9344/85 failed. Any idea of what I did wrong?

Anyway thanks for the fix ;-)

rullzer · 2018-07-31T17:59:24Z

@danxuliu no idea. Maybe the rebase messed things up?
Anyway good to go now I guess :D

danxuliu added bug 3. to review Waiting for reviews feature: users and groups labels Apr 18, 2018

danxuliu added this to the Nextcloud 14 milestone Apr 18, 2018

danxuliu requested review from rullzer, MorrisJobke and skjnldsv April 18, 2018 22:41

MorrisJobke approved these changes Apr 19, 2018

View reviewed changes

MorrisJobke requested review from nickvergessen, ChristophWurst and juliusknorr April 19, 2018 12:33

danxuliu force-pushed the fix-display-name-ignored-when-creating-new-user branch from 4276ce7 to 994a67e Compare April 20, 2018 13:54

MorrisJobke force-pushed the fix-display-name-ignored-when-creating-new-user branch from 994a67e to be6d916 Compare May 23, 2018 18:25

skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels May 26, 2018

MorrisJobke added the stale Ticket or PR with no recent activity label Jun 19, 2018

nextcloud-bot removed the stale Ticket or PR with no recent activity label Jul 4, 2018

MorrisJobke mentioned this pull request Jul 24, 2018

14.0.0 Beta 1 #10370

Merged

21 tasks

MorrisJobke mentioned this pull request Jul 26, 2018

Nextcloud 14 Beta 2 #10410

Closed

51 tasks

danxuliu added 3 commits July 31, 2018 13:33

Add optional "displayName" parameter to add user endpoint

36326e3

Signed-off-by: Daniel Calviño Sánchez <[email protected]>

Add acceptance test for creating a user with a custom display name

029b33a

Signed-off-by: Daniel Calviño Sánchez <[email protected]>

danxuliu force-pushed the fix-display-name-ignored-when-creating-new-user branch from be6d916 to 029b33a Compare July 31, 2018 11:42

danxuliu added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 31, 2018

skjnldsv approved these changes Jul 31, 2018

View reviewed changes

Compile vue

f05d30c

Signed-off-by: Roeland Jago Douma <[email protected]>

rullzer merged commit 411387d into master Jul 31, 2018

rullzer deleted the fix-display-name-ignored-when-creating-new-user branch July 31, 2018 17:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix display name ignored when creating new user #9242

Fix display name ignored when creating new user #9242

danxuliu commented Apr 18, 2018

codecov bot commented Apr 18, 2018 •

edited

Loading

MorrisJobke commented Apr 19, 2018

skjnldsv commented Apr 19, 2018

MorrisJobke left a comment

danxuliu commented Apr 20, 2018

MorrisJobke commented Apr 20, 2018

skjnldsv commented Apr 20, 2018

danxuliu commented Apr 23, 2018

skjnldsv commented Apr 23, 2018 •

edited

Loading

MorrisJobke commented May 7, 2018

skjnldsv commented May 7, 2018

danxuliu commented May 7, 2018

skjnldsv commented May 7, 2018

danxuliu commented May 7, 2018

MorrisJobke commented May 23, 2018

skjnldsv commented May 23, 2018

MorrisJobke commented Jul 4, 2018

skjnldsv commented Jul 4, 2018

danxuliu commented Jul 24, 2018 •

edited

Loading

danxuliu commented Jul 31, 2018

rullzer commented Jul 31, 2018

danxuliu commented Jul 31, 2018

rullzer commented Jul 31, 2018

Fix display name ignored when creating new user #9242

Fix display name ignored when creating new user #9242

Conversation

danxuliu commented Apr 18, 2018

codecov bot commented Apr 18, 2018 • edited Loading

Codecov Report

MorrisJobke commented Apr 19, 2018

skjnldsv commented Apr 19, 2018

MorrisJobke left a comment

Choose a reason for hiding this comment

danxuliu commented Apr 20, 2018

MorrisJobke commented Apr 20, 2018

skjnldsv commented Apr 20, 2018

danxuliu commented Apr 23, 2018

skjnldsv commented Apr 23, 2018 • edited Loading

MorrisJobke commented May 7, 2018

skjnldsv commented May 7, 2018

danxuliu commented May 7, 2018

skjnldsv commented May 7, 2018

danxuliu commented May 7, 2018

MorrisJobke commented May 23, 2018

skjnldsv commented May 23, 2018

MorrisJobke commented Jul 4, 2018

skjnldsv commented Jul 4, 2018

danxuliu commented Jul 24, 2018 • edited Loading

danxuliu commented Jul 31, 2018

rullzer commented Jul 31, 2018

danxuliu commented Jul 31, 2018

rullzer commented Jul 31, 2018

codecov bot commented Apr 18, 2018 •

edited

Loading

skjnldsv commented Apr 23, 2018 •

edited

Loading

danxuliu commented Jul 24, 2018 •

edited

Loading