Merge pull request #9242 from nextcloud/fix-display-name-ignored-when-creating-new-user

Fix display name ignored when creating new user

apps/provisioning_api/lib/Controller/UsersController.php

@@ -199,6 +199,7 @@ class UsersController extends AUserData {
 	 *
 	 * @param string $userid
 	 * @param string $password
+	 * @param string $displayName
 	 * @param string $email
 	 * @param array $groups
 	 * @param array $subadmins
@@ -209,6 +210,7 @@ class UsersController extends AUserData {
 	 */
 	public function addUser(string $userid,
 							string $password = '',
+							string $displayName = '',
 							string $email = '',
 							array $groups = [],
 							array $subadmin = [],
@@ -282,6 +284,10 @@ class UsersController extends AUserData {
 				$subAdminManager->createSubAdmin($newUser, $group);
 			}
 
+			if ($displayName !== '') {
+				$this->editUser($userid, 'display', $displayName);
+			}
+
 			if ($quota !== '') {
 				$this->editUser($userid, 'quota', $quota);
 			}

apps/provisioning_api/tests/Controller/UsersControllerTest.php

@@ -247,7 +247,7 @@ class UsersControllerTest extends TestCase {
 			->with('adminUser')
 			->willReturn(true);
 
-		$this->api->addUser('AlreadyExistingUser', 'password', '', []);
+		$this->api->addUser('AlreadyExistingUser', 'password', '', '', []);
 	}
 
 	/**
@@ -283,7 +283,7 @@ class UsersControllerTest extends TestCase {
 			->with('NonExistingGroup')
 			->willReturn(false);
 
-		$this->api->addUser('NewUser', 'pass', '', ['NonExistingGroup']);
+		$this->api->addUser('NewUser', 'pass', '', '', ['NonExistingGroup']);
 	}
 
 	/**
@@ -325,7 +325,7 @@ class UsersControllerTest extends TestCase {
 				['NonExistingGroup', false]
 			]));
 
-		$this->api->addUser('NewUser', 'pass', '', ['ExistingGroup', 'NonExistingGroup']);
+		$this->api->addUser('NewUser', 'pass', '', '', ['ExistingGroup', 'NonExistingGroup']);
 	}
 
 	public function testAddUserSuccessful() {
@@ -362,6 +362,63 @@ class UsersControllerTest extends TestCase {
 		$this->assertEquals([], $this->api->addUser('NewUser', 'PasswordOfTheNewUser')->getData());
 	}
 
+	public function testAddUserSuccessfulWithDisplayName() {
+		$api = $this->getMockBuilder('OCA\Provisioning_API\Controller\UsersController')
+			->setConstructorArgs([
+				'provisioning_api',
+				$this->request,
+				$this->userManager,
+				$this->config,
+				$this->appManager,
+				$this->groupManager,
+				$this->userSession,
+				$this->accountManager,
+				$this->logger,
+				$this->l10nFactory,
+				$this->newUserMailHelper,
+				$this->federatedFileSharingFactory,
+				$this->secureRandom
+			])
+			->setMethods(['editUser'])
+			->getMock();
+
+		$this->userManager
+			->expects($this->once())
+			->method('userExists')
+			->with('NewUser')
+			->will($this->returnValue(false));
+		$this->userManager
+			->expects($this->once())
+			->method('createUser')
+			->with('NewUser', 'PasswordOfTheNewUser');
+		$this->logger
+			->expects($this->once())
+			->method('info')
+			->with('Successful addUser call with userid: NewUser', ['app' => 'ocs_api']);
+		$loggedInUser = $this->getMockBuilder(IUser::class)
+			->disableOriginalConstructor()
+			->getMock();
+		$loggedInUser
+			->expects($this->any())
+			->method('getUID')
+			->will($this->returnValue('adminUser'));
+		$this->userSession
+			->expects($this->any())
+			->method('getUser')
+			->will($this->returnValue($loggedInUser));
+		$this->groupManager
+			->expects($this->once())
+			->method('isAdmin')
+			->with('adminUser')
+			->willReturn(true);
+		$api
+			->expects($this->once())
+			->method('editUser')
+			->with('NewUser', 'display', 'DisplayNameOfTheNewUser');
+
+		$this->assertEquals([], $api->addUser('NewUser', 'PasswordOfTheNewUser', 'DisplayNameOfTheNewUser')->getData());
+	}
+
 	public function testAddUserExistingGroup() {
 		$this->userManager
 			->expects($this->once())
@@ -417,7 +474,7 @@ class UsersControllerTest extends TestCase {
 				['Added userid NewUser to group ExistingGroup', ['app' => 'ocs_api']]
 			);
 
-		$this->assertEquals([], $this->api->addUser('NewUser', 'PasswordOfTheNewUser', '', ['ExistingGroup'])->getData());
+		$this->assertEquals([], $this->api->addUser('NewUser', 'PasswordOfTheNewUser', '', '', ['ExistingGroup'])->getData());
 	}
 
 	/**
@@ -495,7 +552,7 @@ class UsersControllerTest extends TestCase {
 			->with()
 			->willReturn($subAdminManager);
 
-		$this->api->addUser('NewUser', 'PasswordOfTheNewUser', '', []);
+		$this->api->addUser('NewUser', 'PasswordOfTheNewUser', '', '', []);
 	}
 
 	/**
@@ -544,7 +601,7 @@ class UsersControllerTest extends TestCase {
 			->with('ExistingGroup')
 			->willReturn(true);
 
-		$this->api->addUser('NewUser', 'PasswordOfTheNewUser', '', ['ExistingGroup'])->getData();
+		$this->api->addUser('NewUser', 'PasswordOfTheNewUser', '', '', ['ExistingGroup'])->getData();
 	}
 
 	public function testAddUserAsSubAdminExistingGroups() {
@@ -635,7 +692,7 @@ class UsersControllerTest extends TestCase {
 			)
 			->willReturn(true);
 
-		$this->assertEquals([], $this->api->addUser('NewUser', 'PasswordOfTheNewUser', '', ['ExistingGroup1', 'ExistingGroup2'])->getData());
+		$this->assertEquals([], $this->api->addUser('NewUser', 'PasswordOfTheNewUser', '', '', ['ExistingGroup1', 'ExistingGroup2'])->getData());
 	}
 
 	/**

settings/src/components/userList.vue

@@ -324,6 +324,7 @@ export default {
 			this.$store.dispatch('addUser', {
 				userid: this.newUser.id,
 				password: this.newUser.password,
+				displayName: this.newUser.displayName,
 				email: this.newUser.mailAddress,
 				groups: this.newUser.groups.map(group => group.id),
 				subadmin: this.newUser.subAdminsGroups.map(group => group.id),

settings/src/store/users.js

@@ -415,16 +415,17 @@ const actions = {
 	 * @param {Object} context
 	 * @param {Object} options
 	 * @param {string} options.userid User id
-	 * @param {string} options.password User password 
+	 * @param {string} options.password User password
+	 * @param {string} options.displayName User display name
 	 * @param {string} options.email User email
 	 * @param {string} options.groups User groups
 	 * @param {string} options.subadmin User subadmin groups
 	 * @param {string} options.quota User email
 	 * @returns {Promise}
 	 */
-	addUser({commit, dispatch}, { userid, password, email, groups, subadmin, quota, language }) {
+	addUser({commit, dispatch}, { userid, password, displayName, email, groups, subadmin, quota, language }) {
 		return api.requireAdmin().then((response) => {
-			return api.post(OC.linkToOCS(`cloud/users`, 2), { userid, password, email, groups, subadmin, quota, language })
+			return api.post(OC.linkToOCS(`cloud/users`, 2), { userid, password, displayName, email, groups, subadmin, quota, language })
 				.then((response) => dispatch('addUserData', userid))
 				.catch((error) => {throw error;});
 		}).catch((error) => commit('API_FAILURE', { userid, error }));

tests/acceptance/features/bootstrap/UsersSettingsContext.php

@@ -44,6 +44,14 @@ class UsersSettingsContext implements Context, ActorAwareInterface {
 				describedAs("User name field for new user in Users Settings");
 	}
 
+	/**
+	 * @return Locator
+	 */
+	public static function displayNameFieldForNewUser() {
+		return Locator::forThe()->field("newdisplayname")->
+				describedAs("Display name field for new user in Users Settings");
+	}
+
 	/**
 	 * @return Locator
 	 */
@@ -96,6 +104,13 @@ class UsersSettingsContext implements Context, ActorAwareInterface {
 				describedAs("$cell input for user $user in Users Settings");
 	}
 
+	/**
+	 * @return Locator
+	 */
+	public static function displayNameCellForUser($user) {
+		return self::inputForUserInCell("displayName", $user);
+	}
+
 	/**
 	 * @return Locator
 	 */
@@ -161,6 +176,34 @@ class UsersSettingsContext implements Context, ActorAwareInterface {
 		$this->actor->find(self::actionsMenuOf($user))->click();
 	}
 
+	/**
+	 * @When I set the user name for the new user to :user
+	 */
+	public function iSetTheUserNameForTheNewUserTo($user) {
+		$this->actor->find(self::userNameFieldForNewUser(), 10)->setValue($user);
+	}
+
+	/**
+	 * @When I set the display name for the new user to :displayName
+	 */
+	public function iSetTheDisplayNameForTheNewUserTo($displayName) {
+		$this->actor->find(self::displayNameFieldForNewUser(), 10)->setValue($displayName);
+	}
+
+	/**
+	 * @When I set the password for the new user to :password
+	 */
+	public function iSetThePasswordForTheNewUserTo($password) {
+		$this->actor->find(self::passwordFieldForNewUser(), 10)->setValue($password);
+	}
+
+	/**
+	 * @When I create the new user
+	 */
+	public function iCreateTheNewUser() {
+		$this->actor->find(self::createNewUserButton(), 10)->click();
+	}
+
 	/**
 	 * @When I create user :user with password :password
 	 */
@@ -242,6 +285,13 @@ class UsersSettingsContext implements Context, ActorAwareInterface {
 			$this->actor->find(self::inputForUserInCell($field, $user), 10)->getValue(), $value);
 	}
 
+	/**
+	 * @Then I see that the display name for the user :user is :displayName
+	 */
+	public function iSeeThatTheDisplayNameForTheUserIs($user, $displayName) {
+		PHPUnit_Framework_Assert::assertEquals($displayName, $this->actor->find(self::displayNameCellForUser($user), 10)->getValue());
+	}
+
 	/**
 	 * @Then I see that the :cell cell for user :user is done loading
 	 */

tests/acceptance/features/users.feature

@@ -9,6 +9,18 @@ Feature: users
     When I create user unknownUser with password 123456acb
     Then I see that the list of users contains the user unknownUser
 
+  Scenario: create a new user with a custom display name
+    Given I am logged in as the admin
+    And I open the User settings
+    When I click the New user button
+    And I see that the new user form is shown
+    And I set the user name for the new user to "test"
+    And I set the display name for the new user to "Test display name"
+    And I set the password for the new user to "123456acb"
+    And I create the new user
+    Then I see that the list of users contains the user "test"
+    And I see that the display name for the user "test" is "Test display name"
+
   Scenario: delete a user
     Given I act as Jane
     And I am logged in as the admin