NateWr
commented
7 years ago This issue is addressed in this commit: https://github.com/NateWr/pkp-lib/commit/6ed3c58bb6aaf0030fe1f7883b4e98ca9fee325f
However, it's part of some broader refactoring I'm doing in an attempt to implement an event observer (#2163). That may not get merged. If it doesn't, the fix can be cherry-picked over. But the user grid syncing will need to be accounted for in a different way (it currently uses an event router that only exists in that branch).
(The commit hash above could get changed if I have to rebase. So here's a copy of the full diff.)
commit 6ed3c58bb6aaf0030fe1f7883b4e98ca9fee325f
Author: Nate Wright <natew@notthisway.com>
Date: Wed Jan 11 15:11:49 2017 +0000
pkp/pkp-lib#2163 Refactor user merging to avoid partial handler refresh
Previously, a user tab handler performed a partial tab refresh
to refresh the user grid when a user merge occurred. This
causes problems for the handler event deregistration process
I'm working on implementing. This commit refactors away from
that approach in favor of a modal approach. It also fixed
pkp/pkp-lib#2171 in the process.
diff --git a/controllers/grid/settings/user/UserGridHandler.inc.php b/controllers/grid/settings/user/UserGridHandler.inc.php
index 4d81525..0590407 100644
--- a/controllers/grid/settings/user/UserGridHandler.inc.php
+++ b/controllers/grid/settings/user/UserGridHandler.inc.php
@@ -208,7 +208,9 @@ class UserGridHandler extends GridHandler {
$filterData = array(
'userGroupOptions' => $userGroupOptions,
'fieldOptions' => $fieldOptions,
- 'matchOptions' => $matchOptions
+ 'matchOptions' => $matchOptions,
+ // oldUserId is used when merging users. see: userGridFilter.tpl
+ 'oldUserId' => $request->getUserVar('oldUserId'),
);
return parent::renderFilter($request, $filterData);
@@ -243,6 +245,14 @@ class UserGridHandler extends GridHandler {
return 'controllers/grid/settings/user/userGridFilter.tpl';
}
+ /**
+ * Get the js handler for this component.
+ * @return string
+ */
+ public function getJSHandler() {
+ return '$.pkp.controllers.grid.users.UserGridHandler';
+ }
+
//
// Public grid actions.
@@ -519,21 +529,30 @@ class UserGridHandler extends GridHandler {
* @return JSONMessage JSON object
*/
function mergeUsers($args, $request) {
- if (!$request->checkCSRF()) return new JSONMessage(false);
- // if there is a $newUserId, this is the second time through, so merge the users.
$newUserId = (int) $request->getUserVar('newUserId');
$oldUserId = (int) $request->getUserVar('oldUserId');
$user = $request->getUser();
+
+ // if there is a $newUserId, this is the second time through, so merge the users.
if ($newUserId > 0 && $oldUserId > 0 && Validation::canAdminister($oldUserId, $user->getId())) {
+ if (!$request->checkCSRF()) return new JSONMessage(false);
import('classes.user.UserAction');
$userAction = new UserAction();
$userAction->mergeUsers($oldUserId, $newUserId);
- return DAO::getDataChangedEvent();
+ $json = DAO::getDataChangedEvent();
+ $json->setGlobalEvent('userMerged', array(
+ 'oldUserId' => $oldUserId,
+ 'newUserId' => $newUserId,
+ ));
+ return $json;
+
+ // Otherwise present the grid for selecting the user to merge into
} else {
- // The grid shouldn't have presented an action in this
- // case.
- return new JSONMessage(false, __('grid.user.cannotAdminister'));
+ $userGrid = new UserGridHandler();
+ $userGrid->initialize($request);
+ $userGrid->setTitle('grid.user.mergeUsers.mergeIntoUser');
+ return $userGrid->fetchGrid($args, $request);
}
}
diff --git a/controllers/grid/settings/user/UserGridRow.inc.php b/controllers/grid/settings/user/UserGridRow.inc.php
index a7abaaf..d44dead 100644
--- a/controllers/grid/settings/user/UserGridRow.inc.php
+++ b/controllers/grid/settings/user/UserGridRow.inc.php
@@ -56,133 +56,135 @@ class UserGridRow extends GridRow {
$actionArgs = array_merge($actionArgs, $this->getRequestArgs());
- $this->addAction(
- new LinkAction(
- 'email',
- new AjaxModal(
- $router->url($request, null, null, 'editEmail', null, $actionArgs),
- __('grid.user.email'),
- 'modal_email',
- true
- ),
- __('grid.user.email'),
- 'notify')
- );
- $this->addAction(
- new LinkAction(
- 'edit',
- new AjaxModal(
- $router->url($request, null, null, 'editUser', null, $actionArgs),
- __('grid.user.edit'),
- 'modal_edit',
- true
- ),
- __('grid.user.edit'),
- 'edit')
- );
- if ($element->getDisabled()) {
- $actionArgs['enable'] = true;
+ // If this is the grid for merging a user, only show the merge
+ // linkaction
+ if ($this->getOldUserId()) {
+ $actionArgs['oldUserId'] = $this->getOldUserId();
+ $actionArgs['newUserId'] = $rowId;
+
+ // Verify that the old user exists
+ $userDao = DAORegistry::getDAO('UserDAO');
+ $oldUser = $userDao->getById($this->getOldUserId());
+
+ // Don't merge a user in itself
+ if ($oldUser && $actionArgs['oldUserId'] != $actionArgs['newUserId']) {
+ $this->addAction(
+ new LinkAction(
+ 'mergeUser',
+ new RemoteActionConfirmationModal(
+ $request->getSession(),
+ __('grid.user.mergeUsers.confirm', array('oldUsername' => $oldUser->getUsername(), 'newUsername' => $element->getUsername())),
+ null,
+ $router->url($request, null, null, 'mergeUsers', null, $actionArgs),
+ 'modal_merge_users'
+ ),
+ __('grid.user.mergeUsers.mergeIntoUser'),
+ 'merge_users')
+ );
+ }
+
+ // Otherwise display all the default link actions
+ } else {
+
$this->addAction(
new LinkAction(
- 'enable',
+ 'email',
new AjaxModal(
- $router->url($request, null, null, 'editDisableUser', null, $actionArgs),
- __('common.enable'),
- 'enable',
+ $router->url($request, null, null, 'editEmail', null, $actionArgs),
+ __('grid.user.email'),
+ 'modal_email',
true
),
- __('common.enable'),
- 'enable')
+ __('grid.user.email'),
+ 'notify')
);
- } else {
- $actionArgs['enable'] = false;
$this->addAction(
new LinkAction(
- 'disable',
+ 'edit',
new AjaxModal(
- $router->url($request, null, null, 'editDisableUser', null, $actionArgs),
- __('grid.user.disable'),
- 'disable',
+ $router->url($request, null, null, 'editUser', null, $actionArgs),
+ __('grid.user.edit'),
+ 'modal_edit',
true
),
- __('grid.user.disable'),
- 'disable')
+ __('grid.user.edit'),
+ 'edit')
);
- }
- $this->addAction(
- new LinkAction(
- 'remove',
- new RemoteActionConfirmationModal(
- $request->getSession(),
- __('manager.people.confirmRemove'),
- __('common.remove'),
- $router->url($request, null, null, 'removeUser', null, $actionArgs),
- 'modal_delete'
- ),
- __('grid.action.remove'),
- 'delete')
- );
-
- $canAdminister = Validation::canAdminister($this->getId(), $request->getUser()->getId());
- if (
- !Validation::isLoggedInAs() and
- $request->getUser()->getId() != $this->getId() and
- $canAdminister
- ) {
- $dispatcher = $router->getDispatcher();
+ if ($element->getDisabled()) {
+ $actionArgs['enable'] = true;
+ $this->addAction(
+ new LinkAction(
+ 'enable',
+ new AjaxModal(
+ $router->url($request, null, null, 'editDisableUser', null, $actionArgs),
+ __('common.enable'),
+ 'enable',
+ true
+ ),
+ __('common.enable'),
+ 'enable')
+ );
+ } else {
+ $actionArgs['enable'] = false;
+ $this->addAction(
+ new LinkAction(
+ 'disable',
+ new AjaxModal(
+ $router->url($request, null, null, 'editDisableUser', null, $actionArgs),
+ __('grid.user.disable'),
+ 'disable',
+ true
+ ),
+ __('grid.user.disable'),
+ 'disable')
+ );
+ }
$this->addAction(
new LinkAction(
- 'logInAs',
- new RedirectConfirmationModal(
- __('grid.user.confirmLogInAs'),
- __('grid.action.logInAs'),
- $dispatcher->url($request, ROUTE_PAGE, null, 'login', 'signInAsUser', $this->getId())
- ),
- __('grid.action.logInAs'),
- 'enroll_user'
- )
+ 'remove',
+ new RemoteActionConfirmationModal(
+ $request->getSession(),
+ __('manager.people.confirmRemove'),
+ __('common.remove'),
+ $router->url($request, null, null, 'removeUser', null, $actionArgs),
+ 'modal_delete'
+ ),
+ __('grid.action.remove'),
+ 'delete')
);
- }
-
- $oldUserId = $this->getOldUserId();
- $userDao = DAORegistry::getDAO('UserDAO');
- $oldUser = $userDao->getById($this->getOldUserId());
- if ($oldUser) {
- $actionArgs['oldUserId'] = $this->getOldUserId();
- $actionArgs['newUserId'] = $rowId;
- // Don't merge a user in itself
- if ($actionArgs['oldUserId'] != $actionArgs['newUserId']) {
- $userDao = DAORegistry::getDAO('UserDAO');
- $oldUser = $userDao->getById($this->getOldUserId());
+ $canAdminister = Validation::canAdminister($this->getId(), $request->getUser()->getId());
+ if (
+ !Validation::isLoggedInAs() and
+ $request->getUser()->getId() != $this->getId() and
+ $canAdminister
+ ) {
+ $dispatcher = $router->getDispatcher();
$this->addAction(
new LinkAction(
- 'mergeUser',
- new RemoteActionConfirmationModal(
- $request->getSession(),
- __('grid.user.mergeUsers.confirm', array('oldUsername' => $oldUser->getUsername(), 'newUsername' => $element->getUsername())),
- null,
- $router->url($request, null, null, 'mergeUsers', null, $actionArgs),
- 'modal_merge_users'
+ 'logInAs',
+ new RedirectConfirmationModal(
+ __('grid.user.confirmLogInAs'),
+ __('grid.action.logInAs'),
+ $dispatcher->url($request, ROUTE_PAGE, null, 'login', 'signInAsUser', $this->getId())
),
- __('grid.user.mergeUsers.mergeIntoUser'),
- 'merge_users')
+ __('grid.action.logInAs'),
+ 'enroll_user'
+ )
);
}
- } else {
// do not allow the deletion of the admin account.
if ($rowId > 1 && $canAdminister) {
$this->addAction(
new LinkAction(
'mergeUser',
- new JsEventConfirmationModal(
- __('grid.user.mergeUsers.mergeUserSelect.confirm'),
- 'confirmationModalConfirmed',
- array('oldUserId' => $rowId),
- null,
- 'modal_merge_users'
- ),
+ new AjaxModal(
+ $router->url($request, null, null, 'mergeUsers', null, array('oldUserId' => $rowId)),
+ __('grid.user.mergeUsers.mergeUser'),
+ 'modal_merge_users',
+ true
+ ),
__('grid.user.mergeUsers.mergeUser'),
'merge_users')
);
diff --git a/js/controllers/grid/users/UserGridHandler.js b/js/controllers/grid/users/UserGridHandler.js
new file mode 100644
index 0000000..8b1d8ad
--- /dev/null
+++ b/js/controllers/grid/users/UserGridHandler.js
@@ -0,0 +1,49 @@
+/**
+ * @file js/controllers/grid/users/UserGridHandler.js
+ *
+ * Copyright (c) 2014-2016 Simon Fraser University Library
+ * Copyright (c) 2000-2016 John Willinsky
+ * Distributed under the GNU GPL v2. For full terms see the file docs/COPYING.
+ *
+ * @class UserGridHandler
+ * @ingroup js_controllers_grid
+ *
+ * @brief User grid handler. Used to keep user grids in sync, such as when
+ * merging users.
+ */
+(function($) {
+
+ // Define the namespace.
+ $.pkp.controllers.grid.users = $.pkp.controllers.grid.users || {};
+
+
+ /**
+ * @constructor
+ *
+ * @extends $.pkp.controllers.grid.GridHandler
+ *
+ * @param {jQueryObject} $grid The grid this handler is
+ * attached to.
+ * @param {Object} options Grid handler configuration.
+ */
+ $.pkp.controllers.grid.users.UserGridHandler =
+ function($grid, options) {
+ this.parent($grid, options);
+
+ this.bind('userMerged', function() {
+
+ // Close the modal with the merging user grid
+ // This is a little bit hacky. The `formSubmitted` event is
+ // typically fired by an AjaxFormHandler when a form is submitted,
+ // and AjaxModalHandler listens to it to close.
+ this.trigger('formSubmitted');
+
+ this.refreshGridHandler();
+ });
+ };
+ $.pkp.classes.Helper.inherits($.pkp.controllers.grid.users.UserGridHandler,
+ $.pkp.controllers.grid.GridHandler);
+
+
+/** @param {jQuery} $ jQuery closure. */
+}(jQuery));
diff --git a/js/controllers/tab/settings/managementSettings/UsersAndRolesTabHandler.js b/js/controllers/tab/settings/managementSettings/UsersAndRolesTabHandler.js
deleted file mode 100644
index b5f7317..0000000
--- a/js/controllers/tab/settings/managementSettings/UsersAndRolesTabHandler.js
+++ /dev/null
@@ -1,110 +0,0 @@
-/**
- * @defgroup js_controllers_tab_settings_managementSettings
- */
-/**
- * @file js/controllers/tab/settings/managementSettings/UsersAndRolesTabHandler.js
- *
- * Copyright (c) 2014-2016 Simon Fraser University Library
- * Copyright (c) 2000-2016 John Willinsky
- * Distributed under the GNU GPL v2. For full terms see the file docs/COPYING.
- *
- * @class UsersAndRolesTabHandler
- * @ingroup js_controllers_tab_settings_managementSettings
- *
- * @brief A subclass of TabHandler for handling the users and roles tabs. Adds
- * a listener for grid refreshes, so the tab interface can be reloaded.
- */
-(function($) {
-
- /** @type {Object} */
- $.pkp.controllers.tab.settings.managementSettings =
- $.pkp.controllers.tab.managementSettings || {};
-
-
-
- /**
- * @constructor
- *
- * @extends $.pkp.controllers.TabHandler
- *
- * @param {jQueryObject} $tabs A wrapped HTML element that
- * represents the tabbed interface.
- * @param {Object} options Handler options.
- */
- $.pkp.controllers.tab.settings.managementSettings.UsersAndRolesTabHandler =
- function($tabs, options) {
-
- this.parent($tabs, options);
-
- this.bind('confirmationModalConfirmed',
- this.confirmationModalConfirmedHandler_);
-
- if (options.userGridContentUrl) {
- this.userGridContentUrl_ = options.userGridContentUrl;
- }
- };
- $.pkp.classes.Helper.inherits(
- $.pkp.controllers.tab.settings.managementSettings.UsersAndRolesTabHandler,
- $.pkp.controllers.TabHandler);
-
-
- //
- // Private properties
- //
- /**
- * The URL for retrieving a tab's content.
- * @private
- * @type {string?}
- */
- $.pkp.controllers.tab.settings.managementSettings.UsersAndRolesTabHandler.
- prototype.userGridContentUrl_ = null;
-
-
- //
- // Private methods
- //
- /**
- * This listens for grid refreshes from the users grid.
- *
- * @private
- *
- * @param {HTMLElement} sourceElement The parent DIV element
- * which contains the tabs.
- * @param {Event} event The triggered event (confirmationModalConfirmed).
- * @param {?string} data additional event data.
- */
- $.pkp.controllers.tab.settings.managementSettings.UsersAndRolesTabHandler.
- prototype.confirmationModalConfirmedHandler_ =
- function(sourceElement, event, data) {
-
- var $element, jsonObject, prop;
-
- jsonObject = /** @type {{content: {oldUserId: string}}} */ $.parseJSON(data);
-
- if (this.userGridContentUrl_ && jsonObject.content.oldUserId) {
- $element = this.getHtmlElement();
- $.get(this.userGridContentUrl_ + '&oldUserId=' +
- encodeURIComponent(jsonObject.content.oldUserId), null,
- this.callbackWrapper(this.updateTabsHandler_), 'json');
- }
- };
-
-
- /**
- * A callback to update the tabs on the interface.
- *
- * @private
- *
- * @param {Object} ajaxContext The AJAX request context.
- * @param {Object} data A parsed JSON response object.
- */
- $.pkp.controllers.tab.settings.managementSettings.UsersAndRolesTabHandler.
- prototype.updateTabsHandler_ = function(ajaxContext, data) {
-
- var jsonData = /** @type {{content: string}} */ (this.handleJson(data)),
- $element = this.getHtmlElement(),
- currentTab = $element.find('div').first();
- $(currentTab).replaceWith(jsonData.content);
- };
-/** @param {jQuery} $ jQuery closure. */
-}(jQuery));
diff --git a/js/eventRouter/EventRouter.js b/js/eventRouter/EventRouter.js
index 6e5fbe9..77cfbb8 100644
--- a/js/eventRouter/EventRouter.js
+++ b/js/eventRouter/EventRouter.js
@@ -38,6 +38,9 @@
'issueUnpublished': [
'[id^="component-grid-issues-futureissuegrid-"].pkp_controllers_grid',
],
+ 'userMerged': [
+ '[id^="component-grid-settings-user-usergrid-"].pkp_controllers_grid',
+ ],
};
/**
diff --git a/locale/en_US/grid.xml b/locale/en_US/grid.xml
index 5259ada..a49b0cc 100644
--- a/locale/en_US/grid.xml
+++ b/locale/en_US/grid.xml
@@ -229,12 +229,8 @@
<message key="grid.user.mergeUsers">Merge Users</message>
<message key="grid.user.mergeUsers.mergeUser">Merge User</message>
<message key="grid.user.mergeUsers.mergeIntoUser">Merge into this User</message>
- <message key="grid.user.mergeUsers.into.description">Now, you must select a user to whom to attribute the previous user's authorships, editing assignments, etc.</message>
- <message key="grid.user.mergeUsers.from.description">Select a user to merge into another user account (e.g., when someone has two user accounts). The account selected first will be deleted and its submissions, assignments, etc. will be attributed to the second account.</message>
<message key="grid.user.mergeUsers.allUsers">All Enrolled Users</message>
- <message key="grid.user.mergeUsers.mergeUserSelect.confirm">Are you sure you wish to merge this user with another one? You will choose the next user on the following screen.</message>
<message key="grid.user.mergeUsers.confirm">Are you sure you wish to merge the account with the username "{$oldUsername}" into the account with the username "{$newUsername}"? The account with the username "{$oldUsername}" will not exist afterwards. This action is not reversible.</message>
- <message key="grid.user.mergeUsers.noneEnrolled">No users exist with that role.</message>
<!-- Prepared emails -->
<message key="grid.preparedEmails.title">Prepared Email Templates</message>
diff --git a/styles/controllers/grid/grid.less b/styles/controllers/grid/grid.less
index 6f0d62c..762a93c 100644
--- a/styles/controllers/grid/grid.less
+++ b/styles/controllers/grid/grid.less
@@ -604,6 +604,11 @@
}
}
+// Make emails wrap rather than overflow table column in the users grid
+[id^="component-grid-settings-user-usergrid-"] .gridRow td:last-child .label {
+ word-break: break-word;
+}
+
// TODO check if these styles can be removed
.pkp_controllers_grid {
diff --git a/templates/controllers/grid/settings/user/form/userDetailsForm.tpl b/templates/controllers/grid/settings/user/form/userDetailsForm.tpl
index 0d06977..cb075df 100644
--- a/templates/controllers/grid/settings/user/form/userDetailsForm.tpl
+++ b/templates/controllers/grid/settings/user/form/userDetailsForm.tpl
@@ -54,7 +54,7 @@
</div>
</div>
{/if}
+ <p><span class="formRequired">{translate key="common.requiredField"}</span></p>
{fbvFormButtons}
</div>
</form>
-<p><span class="formRequired">{translate key="common.requiredField"}</span></p>
diff --git a/templates/controllers/grid/settings/user/userGridFilter.tpl b/templates/controllers/grid/settings/user/userGridFilter.tpl
index 013755e..2990667 100644
--- a/templates/controllers/grid/settings/user/userGridFilter.tpl
+++ b/templates/controllers/grid/settings/user/userGridFilter.tpl
@@ -5,25 +5,39 @@
* Copyright (c) 2000-2016 John Willinsky
* Distributed under the GNU GPL v2. For full terms see the file docs/COPYING.
*
- * Filter template for user grid.
+ * @brief Filter template for user grid.
+ *
+ * @uses int|null $filterData.oldUserId The user grid is re-used when merging
+ * users. During the process, an $oldUserId is passed representing the user to
+ * be merged. This is used to distinguish the grid filter IDs. The $oldUserId
+ * must be submitted with the client form in order to ensure that the unique
+ * IDs are used when the grid is refreshed. This ensures the filter form
+ * binds to the correct grid when the filter is submitted and refreshed.
*}
+{assign var=filterId value="userSearchForm"}
+{if $filterData.oldUserId}
+ {assign var=filterId value=$filterId|concat:"-userMerge"}
+{/if}
<script type="text/javascript">
// Attach the form handler to the form.
- $('#userSearchForm').pkpHandler('$.pkp.controllers.form.ClientFormHandler',
+ $('#{$filterId}').pkpHandler('$.pkp.controllers.form.ClientFormHandler',
{ldelim}
trackFormChanges: false
{rdelim}
);
</script>
-<form class="pkp_form filter" id="userSearchForm" action="{url router=$smarty.const.ROUTE_COMPONENT component="grid.settings.user.UserGridHandler" op="fetchGrid"}" method="post">
+<form class="pkp_form filter" id="{$filterId}" action="{url router=$smarty.const.ROUTE_COMPONENT component="grid.settings.user.UserGridHandler" op="fetchGrid"}" method="post">
{csrf}
+ {if $filterData.oldUserId}
+ <input type="hidden" name="oldUserId" value="{$filterData.oldUserId|escape}">
+ {/if}
{fbvFormArea id="userSearchFormArea"}
{fbvFormSection title="common.search" for="search"}
{fbvElement type="text" name="search" id="search" value=$filterSelectionData.search size=$fbvStyles.size.LARGE inline="true"}
{fbvElement type="select" name="userGroup" id="userGroup" from=$filterData.userGroupOptions selected=$filterSelectionData.userGroup size=$fbvStyles.size.SMALL translate=false inline="true"}
{/fbvFormSection}
- {fbvFormSection list=true inline=true}
+ {fbvFormSection list=true}
{if $filterSelectionData.includeNoRole}{assign var="checked" value="checked"}{/if}
{fbvElement type="checkbox" name="includeNoRole" id="includeNoRole" value="1" checked=$checked label="user.noRoles.selectUsersWithoutRoles" translate="true"}
{/fbvFormSection}
Choosing to merge a user under Users & Roles > Users causes the tab to be reloaded. When reloaded, it's "swallowed" it's parent element, and is no longer properly nested in the tab. Switching to the Roles tab causes the roles grid to appear underneath the users grid.