diff --git a/settings/js/users/deleteHandler.js b/settings/js/users/deleteHandler.js index 595d56404d80..179cbc4b34bb 100644 --- a/settings/js/users/deleteHandler.js +++ b/settings/js/users/deleteHandler.js @@ -44,27 +44,6 @@ DeleteHandler.TIMEOUT_MS = 7000; */ DeleteHandler.prototype._timeout = null; -/** - * The function to be called after successfully marking the object for deletion - * @callback markCallback - * @param {string} oid the ID of the specific user or group - */ - -/** - * The function to be called after successful delete. The id of the object will - * be passed as argument. Unsuccessful operations will display an error using - * OC.dialogs, no callback is fired. - * @callback removeCallback - * @param {string} oid the ID of the specific user or group - */ - -/** - * This callback is fired after "undo" was clicked so the consumer can update - * the web interface - * @callback undoCallback - * @param {string} oid the ID of the specific user or group - */ - /** * enabled the notification system. Required for undo UI. * @@ -111,55 +90,14 @@ DeleteHandler.prototype.showNotification = function() { } }; -/** - * hides the Undo Notification - */ -DeleteHandler.prototype.hideNotification = function() { - if(this.notifier !== false) { - $('#notification').removeData(this.notificationDataID); - this.notifier.hide(); - } -}; - /** * initializes the delete operation for a given object id * * @param {string} oid the object id */ DeleteHandler.prototype.mark = function(oid) { - if(this.oidToDelete !== false) { - // passing true to avoid hiding the notification - // twice and causing the second notification - // to disappear immediately - this.deleteEntry(true); - } this.oidToDelete = oid; - this.canceled = false; - this.markCallback(oid); - this.showNotification(); - if (this._timeout) { - clearTimeout(this._timeout); - this._timeout = null; - } - if (DeleteHandler.TIMEOUT_MS > 0) { - this._timeout = window.setTimeout( - _.bind(this.deleteEntry, this), - DeleteHandler.TIMEOUT_MS - ); - } -}; - -/** - * cancels a delete operation - */ -DeleteHandler.prototype.cancel = function() { - if (this._timeout) { - clearTimeout(this._timeout); - this._timeout = null; - } - - this.canceled = true; - this.oidToDelete = false; + this.markCallback(decodeURIComponent(oid)); }; /** @@ -173,7 +111,7 @@ DeleteHandler.prototype.cancel = function() { */ DeleteHandler.prototype.deleteEntry = function(keepNotification) { var deferred = $.Deferred(); - if(this.canceled || this.oidToDelete === false) { + if(this.oidToDelete === false) { return deferred.resolve().promise(); } @@ -182,11 +120,6 @@ DeleteHandler.prototype.deleteEntry = function(keepNotification) { dh.hideNotification(); } - if (this._timeout) { - clearTimeout(this._timeout); - this._timeout = null; - } - var payload = {}; payload[dh.ajaxParamID] = dh.oidToDelete; return $.ajax({ @@ -199,10 +132,9 @@ DeleteHandler.prototype.deleteEntry = function(keepNotification) { //TODO: following line dh.removeCallback(dh.oidToDelete); - dh.canceled = true; }, error: function (jqXHR) { - OC.dialogs.alert(jqXHR.responseJSON.data.message, t('settings', 'Unable to delete {objName}', {objName: dh.oidToDelete})); + OC.dialogs.alert(jqXHR.responseJSON.data.message, t('settings', 'Unable to delete {objName}', {objName: decodeURIComponent(dh.oidToDelete)})); dh.undoCallback(dh.oidToDelete); } diff --git a/settings/js/users/groups.js b/settings/js/users/groups.js index 038c62e9ea71..c731ca913ac5 100644 --- a/settings/js/users/groups.js +++ b/settings/js/users/groups.js @@ -269,22 +269,25 @@ GroupList = { GroupDeleteHandler = new DeleteHandler('/settings/users/groups', 'groupname', GroupList.hide, GroupList.remove); - //configure undo - OC.Notification.hide(); var msg = escapeHTML(t('settings', 'deleted {groupName}', {groupName: '%oid'})) + '' + escapeHTML(t('settings', 'undo')) + ''; GroupDeleteHandler.setNotification(OC.Notification, 'deletegroup', msg, - GroupList.show); + GroupList.remove); //when to mark user for delete $userGroupList.on('click', '.delete', function () { // Call function for handling delete/undo - GroupDeleteHandler.mark(encodeURIComponent(GroupList.getElementGID(this)).toString()); - }); - - //delete a marked user when leaving the page - $(window).on('beforeunload', function () { - GroupDeleteHandler.deleteEntry(); + var groupName = encodeURIComponent(GroupList.getElementGID(this)).toString(); + OC.dialogs.confirm( + t('settings', 'You are about to delete a group. This action can\'t be undone and is permanent. Are you sure that you want to permanently delete {groupName}?', {groupName:decodeURIComponent(groupName)}), + t('settings', 'Delete group'), + function (confirmation) { + if (confirmation) { + GroupDeleteHandler.mark(groupName); + GroupDeleteHandler.deleteEntry(); + } + } + ); }); }, diff --git a/settings/js/users/users.js b/settings/js/users/users.js index e07e42fc2837..ffc40418318b 100644 --- a/settings/js/users/users.js +++ b/settings/js/users/users.js @@ -377,12 +377,16 @@ var UserList = { $userListBody.on('click', '.delete', function () { // Call function for handling delete/undo var uid = UserList.getUID(this); - UserDeleteHandler.mark(uid); - }); - - //delete a marked user when leaving the page - $(window).on('beforeunload', function () { - UserDeleteHandler.deleteEntry(); + OC.dialogs.confirm( + t('settings', 'You are about to delete a user. This action can\'t be undone and is permanent. All user data, files and shares will be deleted. Are you sure that you want to permanently delete {userName}?', {userName: uid}), + t('settings', 'Delete user'), + function (confirmation) { + if (confirmation) { + UserDeleteHandler.mark(uid); + UserDeleteHandler.deleteEntry(); + } + } + ); }); }, update: function (gid, limit) { @@ -912,12 +916,7 @@ $(document).ready(function () { } } - var promise; - if (UserDeleteHandler) { - promise = UserDeleteHandler.deleteEntry(); - } else { - promise = $.Deferred().resolve().promise(); - } + var promise = $.Deferred().resolve().promise(); promise.then(function() { var groups = $('#newuser .groups').data('groups') || []; diff --git a/settings/tests/js/users/deleteHandlerSpec.js b/settings/tests/js/users/deleteHandlerSpec.js index d02489c43ce2..0241f45c8213 100644 --- a/settings/tests/js/users/deleteHandlerSpec.js +++ b/settings/tests/js/users/deleteHandlerSpec.js @@ -48,77 +48,6 @@ describe('DeleteHandler tests', function() { hideNotificationSpy.restore(); clock.restore(); }); - it('shows a notification when marking for delete', function() { - var handler = init(markCallback, removeCallback, undoCallback); - handler.mark('some_uid'); - - expect(showNotificationSpy.calledOnce).toEqual(true); - expect(showNotificationSpy.getCall(0).args[0]).toEqual('removed some_uid entry'); - - expect(markCallback.calledOnce).toEqual(true); - expect(markCallback.getCall(0).args[0]).toEqual('some_uid'); - expect(removeCallback.notCalled).toEqual(true); - expect(undoCallback.notCalled).toEqual(true); - - expect(fakeServer.requests.length).toEqual(0); - }); - it('deletes first entry and reshows notification on second delete', function() { - fakeServer.respondWith(/\/index\.php\/dummyendpoint.php\/some_uid/, [ - 204, - { 'Content-Type': 'application/json' }, - JSON.stringify({status: 'success'}) - ]); - fakeServer.respondWith(/\/index\.php\/dummyendpoint.php\/some_other_uid/, [ - 204, - { 'Content-Type': 'application/json' }, - JSON.stringify({status: 'success'}) - ]); - - var handler = init(markCallback, removeCallback, undoCallback); - handler.mark('some_uid'); - - expect(showNotificationSpy.calledOnce).toEqual(true); - expect(showNotificationSpy.getCall(0).args[0]).toEqual('removed some_uid entry'); - showNotificationSpy.restore(); - showNotificationSpy = sinon.spy(OC.Notification, 'showHtml'); - - handler.mark('some_other_uid'); - - expect(hideNotificationSpy.calledOnce).toEqual(true); - expect(showNotificationSpy.calledOnce).toEqual(true); - expect(showNotificationSpy.getCall(0).args[0]).toEqual('removed some_other_uid entry'); - - expect(markCallback.calledTwice).toEqual(true); - expect(markCallback.getCall(0).args[0]).toEqual('some_uid'); - expect(markCallback.getCall(1).args[0]).toEqual('some_other_uid'); - // called only once, because it is called once the second user is deleted - expect(removeCallback.calledOnce).toEqual(true); - expect(undoCallback.notCalled).toEqual(true); - - // previous one was delete - expect(fakeServer.requests.length).toEqual(1); - var request = fakeServer.requests[0]; - expect(request.url).toEqual(OC.webroot + '/index.php/dummyendpoint.php/some_uid'); - }); - it('automatically deletes after timeout', function() { - fakeServer.respondWith(/\/index\.php\/dummyendpoint.php\/some_uid/, [ - 204, - { 'Content-Type': 'application/json' }, - JSON.stringify({status: 'success'}) - ]); - - var handler = init(markCallback, removeCallback, undoCallback); - handler.mark('some_uid'); - - clock.tick(5000); - // nothing happens yet - expect(fakeServer.requests.length).toEqual(0); - - clock.tick(3000); - expect(fakeServer.requests.length).toEqual(1); - var request = fakeServer.requests[0]; - expect(request.url).toEqual(OC.webroot + '/index.php/dummyendpoint.php/some_uid'); - }); it('deletes when deleteEntry is called', function() { fakeServer.respondWith(/\/index\.php\/dummyendpoint.php\/some_uid/, [ 200, @@ -147,31 +76,6 @@ describe('DeleteHandler tests', function() { var request = fakeServer.requests[0]; expect(request.url).toEqual(OC.webroot + '/index.php/dummyendpoint.php/some_uid%3C%3E%2F%22..%5C'); }); - it('cancels deletion when undo is clicked', function() { - var handler = init(markCallback, removeCallback, undoCallback); - handler.setNotification(OC.Notification, 'dataid', 'removed %oid entry Undo', undoCallback); - handler.mark('some_uid'); - $('#notification .undo').click(); - - expect(undoCallback.calledOnce).toEqual(true); - - // timer was cancelled - clock.tick(10000); - expect(fakeServer.requests.length).toEqual(0); - }); - it('cancels deletion when cancel method is called', function() { - var handler = init(markCallback, removeCallback, undoCallback); - handler.setNotification(OC.Notification, 'dataid', 'removed %oid entry Undo', undoCallback); - handler.mark('some_uid'); - handler.cancel(); - - // not sure why, seems to be by design - expect(undoCallback.notCalled).toEqual(true); - - // timer was cancelled - clock.tick(10000); - expect(fakeServer.requests.length).toEqual(0); - }); it('calls removeCallback after successful server side deletion', function() { fakeServer.respondWith(/\/index\.php\/dummyendpoint.php\/some_uid/, [ 200, diff --git a/tests/acceptance/features/bootstrap/WebUIUsersContext.php b/tests/acceptance/features/bootstrap/WebUIUsersContext.php index e858cb90c1fa..2c0171128fc8 100644 --- a/tests/acceptance/features/bootstrap/WebUIUsersContext.php +++ b/tests/acceptance/features/bootstrap/WebUIUsersContext.php @@ -149,19 +149,31 @@ public function theAdminCreatesAUserUsingWithoutAPasswordTheWebUI( /** * - * @When the administrator deletes the group named :name using the webUI + * @When the administrator deletes the group named :name using the webUI and confirms the deletion using the webUI * * @param string $name * * @return void */ public function theAdminDeletesTheGroupUsingTheWebUI($name) { - $this->usersPage->deleteGroup($name, $this->getSession()); + $this->usersPage->deleteGroup($name, $this->getSession(), true); $this->featureContext->rememberThatGroupIsNotExpectedToExist($name); } /** - * @When the administrator deletes these groups using the webUI: + * @When the administrator deletes the group named :name using the webUI and cancels the deletion using the webUI + * + * @param string $name + * + * @return void + */ + public function theAdminDeletesDoesNotDeleteGroupUsingWebUI($name) { + $this->usersPage->deleteGroup($name, $this->getSession(), false); + $this->featureContext->rememberThatGroupIsNotExpectedToExist($name); + } + + /** + * @When the administrator deletes these groups and confirms the deletion using the webUI: * expects a table of groups with the heading "groupname" * * @param TableNode $table @@ -174,6 +186,20 @@ public function theAdminDeletesTheseGroupsUsingTheWebUI(TableNode $table) { } } + /** + * @When the administrator deletes these groups and and cancels the deletion using the webUI: + * expects a table of groups with the heading "groupname" + * + * @param TableNode $table + * + * @return void + */ + public function theAdminDeletesDoesNotTheseGroupsUsingTheWebUI(TableNode $table) { + foreach ($table as $row) { + $this->theAdminDeletesDoesNotDeleteGroupUsingWebUI($row['groupname']); + } + } + /** * @Then the group name :groupName should be listed on the webUI * @@ -269,14 +295,26 @@ public function theDisabledUserTriesToLogin($username, $password) { } /** - * @When the administrator deletes user :username using the webUI + * @When the administrator deletes user :username using the webUI and confirms the deletion using the webUI * * @param string $username * * @return void */ public function theAdministratorDeletesTheUser($username) { - $this->usersPage->deleteUser($username); + $this->usersPage->deleteUser($username, true); + $this->featureContext->rememberThatUserIsNotExpectedToExist($username); + } + + /** + * @When the administrator deletes user :username using the webUI and cancels the deletion using the webUI + * + * @param string $username + * + * @return void + */ + public function theAdministratorDoesNotDeleteTheUser($username) { + $this->usersPage->deleteUser($username, false); $this->featureContext->rememberThatUserIsNotExpectedToExist($username); } diff --git a/tests/acceptance/features/lib/UserPageElement/GroupList.php b/tests/acceptance/features/lib/UserPageElement/GroupList.php index d4f4a32d14ef..6d94254e67c0 100644 --- a/tests/acceptance/features/lib/UserPageElement/GroupList.php +++ b/tests/acceptance/features/lib/UserPageElement/GroupList.php @@ -43,6 +43,10 @@ class GroupList extends OwncloudPage { protected $addGroupXpath = '//span[text()[normalize-space()="Add Group"]]'; protected $addNewGroupInputBoxId = 'newgroupname'; protected $addNewGroupButtonXpath = '//input[@class="button icon-add"]'; + protected $deleteConfirmButtonXpath + = ".//div[contains(@class, 'oc-dialog-buttonrow twobuttons') and not(ancestor::div[contains(@style,'display: none')])]//button[text()='Yes']"; + protected $deleteNotConfirmButtonXpath + = ".//div[contains(@class, 'oc-dialog-buttonrow twobuttons') and not(ancestor::div[contains(@style,'display: none')])]//button[text()='No']"; /** * sets the NodeElement for the current group list @@ -86,11 +90,12 @@ public function selectGroup($name) { * deletes a group in the UI * * @param string $name + * @param bool $confirm * * @throws ElementNotFoundException * @return void */ - public function deleteGroup($name) { + public function deleteGroup($name, $confirm) { $groupLi = $this->selectGroup($name); $deleteButton = $groupLi->find("xpath", $this->deleteBtnXpath); if ($deleteButton === null) { @@ -101,6 +106,22 @@ public function deleteGroup($name) { ); } $deleteButton->click(); + + if ($confirm) { + $confirmButton = $this->find("xpath", $this->deleteConfirmButtonXpath); + } else { + $confirmButton = $this->find("xpath", $this->deleteNotConfirmButtonXpath); + } + + if ($confirmButton === null) { + $xpathSelector = ($confirm) ? $this->deleteConfirmButtonXpath: $this->deleteNotConfirmButtonXpath; + throw new ElementNotFoundException( + __METHOD__ . + " xpath $xpathSelector " . + "could not find delete confirm button" + ); + } + $confirmButton->click(); } /** diff --git a/tests/acceptance/features/lib/UsersPage.php b/tests/acceptance/features/lib/UsersPage.php index a263aedcb5e5..2df5bdf90333 100644 --- a/tests/acceptance/features/lib/UsersPage.php +++ b/tests/acceptance/features/lib/UsersPage.php @@ -69,6 +69,10 @@ class UsersPage extends OwncloudPage { protected $groupListId = "usergrouplist"; protected $disableUserCheckboxXpath = "//input[@type='checkbox']"; protected $deleteUserBtnXpath = ".//td[@class='remove']/a[@class='action delete']"; + protected $deleteConfirmBtnXpath + = ".//div[contains(@class, 'oc-dialog-buttonrow twobuttons') and not(ancestor::div[contains(@style,'display: none')])]//button[text()='Yes']"; + protected $deleteNotConfirmBtnXpath + = ".//div[contains(@class, 'oc-dialog-buttonrow twobuttons') and not(ancestor::div[contains(@style,'display: none')])]//button[text()='No']"; /** * @param string $username @@ -486,12 +490,13 @@ public function getAllGroups() { * * @param string $name * @param Session $session + * @param bool $confirm, true is to delete and false is not to delete * * @return void */ - public function deleteGroup($name, Session $session) { + public function deleteGroup($name, Session $session, $confirm = false) { $groupList = $this->getGroupListElement(); - $groupList->deleteGroup($name); + $groupList->deleteGroup($name, $confirm); $this->waitForAjaxCallsToStartAndFinish($session); } @@ -522,10 +527,11 @@ public function disableUser($username) { /** * * @param string $username + * @param bool $confirm * * @return void */ - public function deleteUser($username) { + public function deleteUser($username, $confirm) { $userTr = $this->findUserInTable($username); $deleteBtn = $userTr->find("xpath", $this->deleteUserBtnXpath); if ($deleteBtn === null) { @@ -536,5 +542,22 @@ public function deleteUser($username) { ); } $deleteBtn->click(); + + if ($confirm) { + $confirmBtn = $this->find('xpath', $this->deleteConfirmBtnXpath); + } else { + $confirmBtn = $this->find('xpath', $this->deleteNotConfirmBtnXpath); + } + + if ($confirmBtn === null) { + $xpathSelector = ($confirm) ? $this->deleteConfirmBtnXpath : $this->deleteNotConfirmBtnXpath; + throw new ElementNotFoundException( + __METHOD__ . + " xpath $xpathSelector " . + "could not find delete confirm button" + ); + } + + $confirmBtn->click(); } } diff --git a/tests/acceptance/features/webUIManageUsersGroups/addUsers.feature b/tests/acceptance/features/webUIManageUsersGroups/addUsers.feature index 88eac9fcacdc..793c8bf9d12e 100644 --- a/tests/acceptance/features/webUIManageUsersGroups/addUsers.feature +++ b/tests/acceptance/features/webUIManageUsersGroups/addUsers.feature @@ -112,7 +112,7 @@ Feature: add users Scenario: recreating a user with same name after deletion sends a new token to new address When the administrator creates a user with the name "guiusr1" and the email "mistake@owncloud" without a password using the webUI - And the administrator deletes user "guiusr1" using the webUI + And the administrator deletes user "guiusr1" using the webUI and confirms the deletion using the webUI And the administrator creates a user with the name "guiusr1" and the email "correct@owncloud" without a password using the webUI And the administrator logs out of the webUI And the user follows the password set link received by "correct@owncloud" using the webUI @@ -122,7 +122,7 @@ Feature: add users Scenario: recreating a user with same name after deletion makes the first token invalid When the administrator creates a user with the name "guiusr1" and the email "mistake@owncloud" without a password using the webUI - And the administrator deletes user "guiusr1" using the webUI + And the administrator deletes user "guiusr1" using the webUI and confirms the deletion using the webUI And the administrator creates a user with the name "guiusr1" and the email "correct@owncloud" without a password using the webUI And the administrator logs out of the webUI And the user follows the password set link received by "mistake@owncloud" using the webUI @@ -131,7 +131,7 @@ Feature: add users Scenario: when recreating a user with same second token can be used even if someone tried to use the first one When the administrator creates a user with the name "guiusr1" and the email "mistake@owncloud" without a password using the webUI - And the administrator deletes user "guiusr1" using the webUI + And the administrator deletes user "guiusr1" using the webUI and confirms the deletion using the webUI And the administrator creates a user with the name "guiusr1" and the email "correct@owncloud" without a password using the webUI And the administrator logs out of the webUI And the user follows the password set link received by "mistake@owncloud" using the webUI diff --git a/tests/acceptance/features/webUIManageUsersGroups/deleteUsers.feature b/tests/acceptance/features/webUIManageUsersGroups/deleteUsers.feature index 2f0a290d7c32..b2fe9c8d3537 100644 --- a/tests/acceptance/features/webUIManageUsersGroups/deleteUsers.feature +++ b/tests/acceptance/features/webUIManageUsersGroups/deleteUsers.feature @@ -13,9 +13,13 @@ Feature: delete users And the administrator has browsed to the users page Scenario: use the webUI to delete a simple user - When the administrator deletes user "user1" using the webUI + When the administrator deletes user "user1" using the webUI and confirms the deletion using the webUI And the deleted user "user1" tries to login using the password "%regular%" using the webUI Then the user should be redirected to a webUI page with the title "%productname%" When the user has browsed to the login page And user "user2" logs in using the webUI Then the user should be redirected to a webUI page with the title "Files - %productname%" + + Scenario: use the webUI to cancel deletion of user + When the administrator deletes user "user1" using the webUI and cancels the deletion using the webUI + Then user "user1" should exist diff --git a/tests/acceptance/features/webUIManageUsersGroups/manageGroups.feature b/tests/acceptance/features/webUIManageUsersGroups/manageGroups.feature index 7f0a9c2e1e53..47c383908361 100644 --- a/tests/acceptance/features/webUIManageUsersGroups/manageGroups.feature +++ b/tests/acceptance/features/webUIManageUsersGroups/manageGroups.feature @@ -17,7 +17,7 @@ Feature: manage groups | false | | do-not-delete2 | And the administrator has browsed to the users page - When the administrator deletes these groups using the webUI: + When the administrator deletes these groups and confirms the deletion using the webUI: | groupname | | 0 | | false | @@ -50,7 +50,7 @@ Feature: manage groups | q?mark | | do-not-delete2 | And the administrator has browsed to the users page - When the administrator deletes these groups using the webUI: + When the administrator deletes these groups and confirms the deletion using the webUI: | groupname | | a/slash | | per%cent | @@ -88,7 +88,7 @@ Feature: manage groups | quotes" | | do-not-delete2 | And the administrator has browsed to the users page - When the administrator deletes these groups using the webUI: + When the administrator deletes these groups and confirms the deletion using the webUI: | groupname | | grp1 | | space group | @@ -115,3 +115,45 @@ Feature: manage groups | space group | | quotes' | | quotes" | + + Scenario: cancel deletion of a group + Given these groups have been created: + | groupname | + | do-not-delete | + | grp1 | + | space group | + | quotes' | + | quotes" | + | do-not-delete2 | + And the administrator has browsed to the users page + When the administrator deletes the group named "space group" using the webUI and cancels the deletion using the webUI + And the administrator reloads the users page + Then group "space group" should exist + + Scenario: cancel deletion of groups + Given these groups have been created: + | groupname | + | do-not-delete | + | grp1 | + | space group | + | quotes' | + | quotes" | + | do-not-delete2 | + | a\slash | + And the administrator has browsed to the users page + When the administrator deletes these groups and and cancels the deletion using the webUI: + | groupname | + | grp1 | + | space group | + | quotes' | + | a\slash | + And the administrator reloads the users page + Then these groups should be listed on the webUI: + | groupname | + | do-not-delete | + | grp1 | + | space group | + | quotes' | + | quotes" | + | do-not-delete2 | + | a\slash |