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

Fix display name ignored when creating new user #9242

Merged
merged 4 commits into from
Jul 31, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions apps/provisioning_api/lib/Controller/UsersController.php
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ public function getUsersDetails(string $search = '', $limit = null, $offset = 0)
*
* @param string $userid
* @param string $password
* @param string $displayName
* @param string $email
* @param array $groups
* @param array $subadmins
Expand All @@ -209,6 +210,7 @@ public function getUsersDetails(string $search = '', $limit = null, $offset = 0)
*/
public function addUser(string $userid,
string $password = '',
string $displayName = '',
string $email = '',
array $groups = [],
array $subadmin = [],
Expand Down Expand Up @@ -282,6 +284,10 @@ public function addUser(string $userid,
$subAdminManager->createSubAdmin($newUser, $group);
}

if ($displayName !== '') {
$this->editUser($userid, 'display', $displayName);
}

if ($quota !== '') {
$this->editUser($userid, 'quota', $quota);
}
Expand Down
71 changes: 64 additions & 7 deletions apps/provisioning_api/tests/Controller/UsersControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ public function testAddUserAlreadyExisting() {
->with('adminUser')
->willReturn(true);

$this->api->addUser('AlreadyExistingUser', 'password', '', []);
$this->api->addUser('AlreadyExistingUser', 'password', '', '', []);
}

/**
Expand Down Expand Up @@ -283,7 +283,7 @@ public function testAddUserNonExistingGroup() {
->with('NonExistingGroup')
->willReturn(false);

$this->api->addUser('NewUser', 'pass', '', ['NonExistingGroup']);
$this->api->addUser('NewUser', 'pass', '', '', ['NonExistingGroup']);
}

/**
Expand Down Expand Up @@ -325,7 +325,7 @@ public function testAddUserExistingGroupNonExistingGroup() {
['NonExistingGroup', false]
]));

$this->api->addUser('NewUser', 'pass', '', ['ExistingGroup', 'NonExistingGroup']);
$this->api->addUser('NewUser', 'pass', '', '', ['ExistingGroup', 'NonExistingGroup']);
}

public function testAddUserSuccessful() {
Expand Down Expand Up @@ -362,6 +362,63 @@ public function testAddUserSuccessful() {
$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())
Expand Down Expand Up @@ -417,7 +474,7 @@ public function testAddUserExistingGroup() {
['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());
}

/**
Expand Down Expand Up @@ -495,7 +552,7 @@ public function testAddUserAsSubAdminNoGroup() {
->with()
->willReturn($subAdminManager);

$this->api->addUser('NewUser', 'PasswordOfTheNewUser', '', []);
$this->api->addUser('NewUser', 'PasswordOfTheNewUser', '', '', []);
}

/**
Expand Down Expand Up @@ -544,7 +601,7 @@ public function testAddUserAsSubAdminValidGroupNotSubAdmin() {
->with('ExistingGroup')
->willReturn(true);

$this->api->addUser('NewUser', 'PasswordOfTheNewUser', '', ['ExistingGroup'])->getData();
$this->api->addUser('NewUser', 'PasswordOfTheNewUser', '', '', ['ExistingGroup'])->getData();
}

public function testAddUserAsSubAdminExistingGroups() {
Expand Down Expand Up @@ -635,7 +692,7 @@ public function testAddUserAsSubAdminExistingGroups() {
)
->willReturn(true);

$this->assertEquals([], $this->api->addUser('NewUser', 'PasswordOfTheNewUser', '', ['ExistingGroup1', 'ExistingGroup2'])->getData());
$this->assertEquals([], $this->api->addUser('NewUser', 'PasswordOfTheNewUser', '', '', ['ExistingGroup1', 'ExistingGroup2'])->getData());
}

/**
Expand Down
16 changes: 8 additions & 8 deletions settings/js/settings-vue.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion settings/js/settings-vue.js.map

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions settings/src/components/userList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
7 changes: 4 additions & 3 deletions settings/src/store/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }));
Expand Down
50 changes: 50 additions & 0 deletions tests/acceptance/features/bootstrap/UsersSettingsContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ public static function userNameFieldForNewUser() {
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
*/
Expand Down Expand Up @@ -96,6 +104,13 @@ public static function inputForUserInCell($cell, $user) {
describedAs("$cell input for user $user in Users Settings");
}

/**
* @return Locator
*/
public static function displayNameCellForUser($user) {
return self::inputForUserInCell("displayName", $user);
}

/**
* @return Locator
*/
Expand Down Expand Up @@ -161,6 +176,34 @@ public function iOpenTheActionsMenuOf($user) {
$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
*/
Expand Down Expand Up @@ -242,6 +285,13 @@ public function iSeeThatTheFieldOfUserIs($field, $user, $value) {
$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
*/
Expand Down
12 changes: 12 additions & 0 deletions tests/acceptance/features/users.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down