From e090da686b5c26305ccf7930797eee7a4aa953e2 Mon Sep 17 00:00:00 2001 From: Montek Date: Sun, 12 Jul 2026 18:52:39 -0400 Subject: [PATCH 1/4] [issue_tracker] add batch editing for issue fields --- .../css/issue_tracker_batchmode.css | 70 +++ .../jsx/IssueTrackerBatchMode.js | 305 +++++++++- .../issue_tracker/jsx/issueTrackerIndex.js | 5 + .../issue_tracker/locale/issue_tracker.pot | 44 ++ modules/issue_tracker/php/batchedit.class.inc | 568 ++++++++++++++++++ modules/issue_tracker/php/edit.class.inc | 20 + .../test/issue_tracker_test_plan.md | 11 + 7 files changed, 1009 insertions(+), 14 deletions(-) create mode 100644 modules/issue_tracker/php/batchedit.class.inc diff --git a/modules/issue_tracker/css/issue_tracker_batchmode.css b/modules/issue_tracker/css/issue_tracker_batchmode.css index 3d42012731..e74e0e0ef3 100644 --- a/modules/issue_tracker/css/issue_tracker_batchmode.css +++ b/modules/issue_tracker/css/issue_tracker_batchmode.css @@ -211,3 +211,73 @@ margin-left: auto; margin-right: 10px; } + +.batch-selection-controls { + display: flex; + align-items: center; + flex-wrap: wrap; + gap: 15px; + padding: 10px; +} + +.batch-selection-controls label { + display: flex; + align-items: center; + margin: 0; +} + +.batch-edit-controls { + display: grid; + grid-template-columns: repeat(4, minmax(150px, 1fr)) auto; + align-items: end; + gap: 15px; + padding: 10px; +} + +.batch-edit-controls label { + margin: 0; +} + +.batch-edit-controls select { + margin-top: 5px; +} + +.batch-update-button { + white-space: nowrap; +} + +.issue-selection-card { + border: 2px solid transparent; + border-radius: 8px; +} + +.issue-selection-card.selected { + border-color: #074785; +} + +.issue-selection-control { + display: flex; + align-items: center; + margin: 0; + padding: 8px 12px; + background-color: #f8f8f8; + border: 1px solid #ddd; + border-bottom: 0; + border-radius: 8px 8px 0 0; +} + +.issue-selection-card .issue-card { + border-radius: 0 0 8px 8px; +} + +@media (max-width: 900px) { + .batch-edit-controls { + grid-template-columns: repeat(2, minmax(150px, 1fr)); + } +} + +@media (max-width: 500px) { + .batch-edit-controls { + grid-template-columns: 1fr; + } +} diff --git a/modules/issue_tracker/jsx/IssueTrackerBatchMode.js b/modules/issue_tracker/jsx/IssueTrackerBatchMode.js index 05ed66e168..098c37e21d 100644 --- a/modules/issue_tracker/jsx/IssueTrackerBatchMode.js +++ b/modules/issue_tracker/jsx/IssueTrackerBatchMode.js @@ -5,17 +5,21 @@ import Loader from 'Loader'; import PaginationLinks from 'jsx/PaginationLinks'; import Panel from 'jsx/Panel'; import {Tabs, TabPane} from 'jsx/Tabs'; +import swal from 'sweetalert2'; import '../css/issue_tracker_batchmode.css'; import {withTranslation} from 'react-i18next'; +const NO_CHANGE = '__no_change__'; + /** * IssueTrackerBatchMode component * * @param {object} props - The component props * @param {object} props.options - The options for the IssueTrackerBatchMode + * @param {boolean} props.canCloseIssues - Whether issues can be closed * @param {function} props.t - Translation function */ -function IssueTrackerBatchMode({options = {}, t}) { +function IssueTrackerBatchMode({options = {}, canCloseIssues, t}) { const [issues, setIssues] = useState([]); const [selectedCategories, setSelectedCategories] = useState([]); const [selectedPriorities, setSelectedPriorities] = useState([]); @@ -27,6 +31,14 @@ function IssueTrackerBatchMode({options = {}, t}) { const [error, setError] = useState(null); const [assignees, setAssignees] = useState({}); const [otherWatchers, setOtherWatchers] = useState({}); + const [selectedIssueIDs, setSelectedIssueIDs] = useState([]); + const [batchUpdates, setBatchUpdates] = useState({ + status: NO_CHANGE, + priority: NO_CHANGE, + category: NO_CHANGE, + assignee: NO_CHANGE, + }); + const [isSubmitting, setIsSubmitting] = useState(false); // Pagination state const [page, setPage] = useState({ @@ -71,16 +83,20 @@ function IssueTrackerBatchMode({options = {}, t}) { const data = await response.json(); // ordering watchers - const orderedWatchers = Object.keys(data.otherWatchers) + const watchers = data.otherWatchers || {}; + const orderedWatchers = Object.keys(watchers) .sort() .reduce((obj, key) => { - obj[key] = data.otherWatchers[key]; + obj[key] = watchers[key]; return obj; }, {} ); // set data - setIssues(data.issues || []); + setIssues((data.issues || []).map((issue) => ({ + ...issue, + issueID: Number(issue.issueID), + }))); setAssignees(data.assignees || {}); setOtherWatchers(orderedWatchers || {}); setIsLoading(false); @@ -143,6 +159,118 @@ function IssueTrackerBatchMode({options = {}, t}) { fetchIssues(); } + /** + * Toggles whether an issue is selected for the batch update. + * + * @param {number} issueID - The issue ID to toggle + */ + function toggleIssueSelection(issueID) { + setSelectedIssueIDs((current) => current.includes(issueID) ? + current.filter((id) => id !== issueID) : + [...current, issueID] + ); + } + + /** + * Selects or clears every issue matching the current filters. + */ + function toggleAllFilteredIssues() { + const filteredIssueIDs = filteredIssues.map((issue) => issue.issueID); + const allFilteredSelected = filteredIssueIDs.length > 0 && + filteredIssueIDs.every((issueID) => selectedIssueIDs.includes(issueID)); + + setSelectedIssueIDs((current) => allFilteredSelected ? + current.filter((issueID) => !filteredIssueIDs.includes(issueID)) : + [...new Set([...current, ...filteredIssueIDs])] + ); + } + + /** + * Updates one field in the pending batch changes. + * + * @param {string} field - The issue field to update + * @param {string} value - The selected value + */ + function updateBatchField(field, value) { + setBatchUpdates((current) => ({...current, [field]: value})); + } + + /** + * Submits the selected batch changes. + */ + async function submitBatchEdit() { + if (selectedIssueIDs.length === 0) { + await swal.fire({ + type: 'info', + text: t('Select at least one issue.', {ns: 'issue_tracker'}), + }); + return; + } + + const updates = Object.entries(batchUpdates).reduce( + (result, [field, value]) => { + if (value !== NO_CHANGE) { + result[field] = value === '' ? null : value; + } + return result; + }, + {} + ); + + if (Object.keys(updates).length === 0) { + await swal.fire({ + type: 'info', + text: t('Choose at least one field to update.', + {ns: 'issue_tracker'}), + }); + return; + } + + setIsSubmitting(true); + try { + const response = await fetch( + `${loris.BaseURL}/issue_tracker/BatchEdit/`, + { + method: 'POST', + credentials: 'include', + headers: {'Content-Type': 'application/json'}, + body: JSON.stringify({issueIDs: selectedIssueIDs, updates}), + } + ); + const data = await response.json(); + if (!response.ok) { + throw new Error(data.error || 'Network response was not ok'); + } + + setSelectedIssueIDs([]); + setBatchUpdates({ + status: NO_CHANGE, + priority: NO_CHANGE, + category: NO_CHANGE, + assignee: NO_CHANGE, + }); + await fetchIssues(); + await swal.fire({ + type: 'success', + text: t('{{count}} issue updated successfully', { + ns: 'issue_tracker', + count: data.updated, + }), + }); + } catch (error) { + console.error('Error updating issues:', error); + await swal.fire({ + type: 'error', + text: error.message || t( + 'Failed to update issues. Please try again later.', + {ns: 'issue_tracker'} + ), + }); + } finally { + setIsSubmitting(false); + } + } + // Pagination functions /** * @@ -173,6 +301,11 @@ function IssueTrackerBatchMode({options = {}, t}) { const startIndex = (page.number - 1) * page.rows; const endIndex = startIndex + page.rows; const paginatedIssues = filteredIssues.slice(startIndex, endIndex); + const filteredIssueIDs = filteredIssues.map((issue) => issue.issueID); + const allFilteredSelected = filteredIssueIDs.length > 0 && + filteredIssueIDs.every((issueID) => selectedIssueIDs.includes(issueID)); + const hasBatchUpdates = Object.values(batchUpdates) + .some((value) => value !== NO_CHANGE); const tabList = [ { @@ -354,6 +487,135 @@ function IssueTrackerBatchMode({options = {}, t}) {
+ +
+ + + {t('{{count}} issue selected', { + ns: 'issue_tracker', + count: selectedIssueIDs.length, + })} + + +
+
+ + + + + +
+ +
{t('{{count}} issues displayed of {{total}}', { @@ -389,17 +651,31 @@ function IssueTrackerBatchMode({options = {}, t}) {
{paginatedIssues.length > 0 ? ( paginatedIssues.map((issue) => ( - + className={'issue-selection-card' + + (selectedIssueIDs.includes(issue.issueID) ? ' selected' : '')} + > + + +
)) ) : (
@@ -452,6 +728,7 @@ IssueTrackerBatchMode.propTypes = { sites: PropTypes.object, assignees: PropTypes.object, }).isRequired, + canCloseIssues: PropTypes.bool.isRequired, t: PropTypes.func.isRequired, }; diff --git a/modules/issue_tracker/jsx/issueTrackerIndex.js b/modules/issue_tracker/jsx/issueTrackerIndex.js index 5bb02a534b..722bb9796e 100644 --- a/modules/issue_tracker/jsx/issueTrackerIndex.js +++ b/modules/issue_tracker/jsx/issueTrackerIndex.js @@ -374,6 +374,11 @@ class IssueTrackerIndex extends Component { this.props.hasPermission(permission))} options={{ priorities: this.state.data.fieldOptions.priorities, statuses: this.state.data.fieldOptions.statuses, diff --git a/modules/issue_tracker/locale/issue_tracker.pot b/modules/issue_tracker/locale/issue_tracker.pot index 9f38a9a8e9..ae528e4c7d 100644 --- a/modules/issue_tracker/locale/issue_tracker.pot +++ b/modules/issue_tracker/locale/issue_tracker.pot @@ -100,6 +100,50 @@ msgstr "" msgid "Batch Edit" msgstr "" +msgid "Batch changes" +msgstr "" + +msgid "Select all filtered issues" +msgstr "" + +msgid "{{count}} issue selected" +msgid_plural "{{count}} issues selected" +msgstr[0] "" + +msgid "Clear selection" +msgstr "" + +msgid "No change" +msgstr "" + +msgid "Uncategorized" +msgstr "" + +msgid "Unassigned" +msgstr "" + +msgid "Apply changes" +msgstr "" + +msgid "Updating..." +msgstr "" + +msgid "Select issue" +msgstr "" + +msgid "Select at least one issue." +msgstr "" + +msgid "Choose at least one field to update." +msgstr "" + +msgid "{{count}} issue updated successfully" +msgid_plural "{{count}} issues updated successfully" +msgstr[0] "" + +msgid "Failed to update issues. Please try again later." +msgstr "" + msgid "Add Comment" msgstr "" diff --git a/modules/issue_tracker/php/batchedit.class.inc b/modules/issue_tracker/php/batchedit.class.inc new file mode 100644 index 0000000000..8d13fcf066 --- /dev/null +++ b/modules/issue_tracker/php/batchedit.class.inc @@ -0,0 +1,568 @@ +getMethod() !== 'POST') { + return new \LORIS\Http\Response\JSON\MethodNotAllowed( + $this->allowedMethods() + ); + } + + $user = $request->getAttribute('user'); + if (!$user instanceof \User + || !$user->hasPermission('issue_tracker_all_issue') + ) { + return new \LORIS\Http\Response\JSON\Forbidden( + 'User does not have permission to batch edit issues.' + ); + } + + $values = json_decode((string)$request->getBody(), true); + if (!is_array($values)) { + return new \LORIS\Http\Response\JSON\BadRequest( + 'Invalid request.' + ); + } + + $issueIDs = $this->_validateIssueIDs($values['issueIDs'] ?? null); + if ($issueIDs instanceof ResponseInterface) { + return $issueIDs; + } + + $updates = $this->_validateUpdates($values['updates'] ?? null, $user); + if ($updates instanceof ResponseInterface) { + return $updates; + } + + return $this->_updateIssues($issueIDs, $updates, $user); + } + + /** + * Validate and normalize the selected issue IDs. + * + * @param mixed $issueIDs The submitted issue IDs. + * + * @return int[]|ResponseInterface Valid issue IDs or an error response. + */ + private function _validateIssueIDs(mixed $issueIDs) : array|ResponseInterface + { + if (!is_array($issueIDs) || empty($issueIDs)) { + return new \LORIS\Http\Response\JSON\BadRequest( + 'At least one issue must be selected.' + ); + } + + foreach ($issueIDs as $issueID) { + if (!is_int($issueID) || $issueID <= 0) { + return new \LORIS\Http\Response\JSON\BadRequest( + 'Issue IDs must be positive integers.' + ); + } + } + + return array_values(array_unique($issueIDs)); + } + + /** + * Validate and normalize the fields to update. + * + * @param mixed $updates The submitted field updates. + * @param \User $user The user making the request. + * + * @return array|ResponseInterface Valid updates or an error + * response. + */ + private function _validateUpdates( + mixed $updates, + \User $user + ) : array|ResponseInterface { + if (!is_array($updates) || empty($updates)) { + return new \LORIS\Http\Response\JSON\BadRequest( + 'At least one field must be updated.' + ); + } + + $unknownFields = array_diff(array_keys($updates), self::EDITABLE_FIELDS); + if (!empty($unknownFields)) { + return new \LORIS\Http\Response\JSON\BadRequest( + 'Unsupported field in batch edit request.' + ); + } + + $db = $this->loris->getDatabaseConnection(); + + if (array_key_exists('status', $updates)) { + if (!is_string($updates['status']) + || !in_array($updates['status'], self::STATUSES, true) + ) { + return new \LORIS\Http\Response\JSON\BadRequest( + 'Invalid issue status.' + ); + } + } + + if (array_key_exists('priority', $updates) + && (!is_string($updates['priority']) + || !in_array($updates['priority'], self::PRIORITIES, true)) + ) { + return new \LORIS\Http\Response\JSON\BadRequest( + 'Invalid issue priority.' + ); + } + + if (array_key_exists('category', $updates)) { + $category = $updates['category']; + if ($category !== null + && (!is_string($category) || $category === '') + ) { + return new \LORIS\Http\Response\JSON\BadRequest( + 'Invalid issue category.' + ); + } + if ($category !== null + && $db->pselectOne( + 'SELECT categoryName FROM issues_categories ' + . 'WHERE categoryName=:category', + ['category' => $category] + ) === null + ) { + return new \LORIS\Http\Response\JSON\BadRequest( + 'Invalid issue category.' + ); + } + } + + if (array_key_exists('assignee', $updates)) { + $assignee = $updates['assignee']; + if ($assignee !== null + && (!is_string($assignee) || $assignee === '') + ) { + return new \LORIS\Http\Response\JSON\BadRequest( + 'Invalid issue assignee.' + ); + } + if ($assignee !== null + && $db->pselectOne( + "SELECT UserID FROM users + WHERE UserID=:assignee AND Pending_approval='N'", + ['assignee' => $assignee] + ) === null + ) { + return new \LORIS\Http\Response\JSON\BadRequest( + 'Invalid issue assignee.' + ); + } + } + + return $updates; + } + + /** + * Apply the validated updates to each selected issue. + * + * @param int[] $issueIDs The issue IDs to update. + * @param array $updates The fields to update. + * @param \User $user The user making the request. + * + * @return ResponseInterface The batch edit result. + */ + private function _updateIssues( + array $issueIDs, + array $updates, + \User $user + ) : ResponseInterface { + $db = $this->loris->getDatabaseConnection(); + $placeholders = implode(',', array_fill(0, count($issueIDs), '?')); + + try { + $db->beginTransaction(); + /** + * Selected issue rows. + * + * @var array $issues + */ + $issues = iterator_to_array( + $db->pselect( + "SELECT issueID, assignee, status, priority, category, + lastUpdatedBy, centerID, reporter + FROM issues + WHERE issueID IN ($placeholders) + FOR UPDATE", + $issueIDs + ) + ); + + if (count($issues) !== count($issueIDs)) { + $db->rollBack(); + return new \LORIS\Http\Response\JSON\BadRequest( + 'One or more selected issues do not exist.' + ); + } + + if (isset($updates['status']) + && in_array($updates['status'], self::CLOSED_STATUSES, true) + ) { + foreach ($issues as $issue) { + if (!$this->_canCloseIssue($issue, $user)) { + $db->rollBack(); + return new \LORIS\Http\Response\JSON\Forbidden( + 'User does not have permission to close one or more ' + . 'selected issues.' + ); + } + } + } + + $globalWatchers = $this->_getGlobalWatchers(); + /** + * Pending issue notifications. + * + * @var array $notificationQueue + */ + $notificationQueue = []; + $updatedCount = 0; + foreach ($issues as $issue) { + $changedValues = []; + foreach ($updates as $field => $value) { + if ($issue[$field] !== $value) { + $changedValues[$field] = $value; + } + } + + if (empty($changedValues)) { + continue; + } + + $issueID = (int)$issue['issueID']; + if ($issue['lastUpdatedBy'] !== $user->getUsername()) { + $changedValues['lastUpdatedBy'] = $user->getUsername(); + } + + $db->unsafeUpdate('issues', $changedValues, ['issueID' => $issueID]); + + foreach ($changedValues as $field => $value) { + $db->unsafeInsert( + 'issues_history', + [ + 'newValue' => $value ?? '', + 'fieldChanged' => $field, + 'issueID' => $issueID, + 'addedBy' => $user->getUsername(), + ] + ); + } + + $newAssignee = $issue['assignee']; + if (array_key_exists('assignee', $changedValues)) { + $newAssignee = $changedValues['assignee']; + if ($issue['assignee'] !== null) { + $db->delete( + 'issues_watching', + [ + 'userID' => $issue['assignee'], + 'issueID' => $issueID, + ] + ); + } + if ($newAssignee !== null) { + $db->replace( + 'issues_watching', + ['userID' => $newAssignee, 'issueID' => $issueID] + ); + } + } + + foreach ($globalWatchers as $watcher) { + $db->replace( + 'issues_watching', + ['userID' => $watcher, 'issueID' => $issueID] + ); + } + + $notificationQueue[] = [ + 'issueID' => $issueID, + 'oldAssignee' => $issue['assignee'], + 'newAssignee' => $newAssignee, + ]; + $updatedCount++; + } + + $db->commit(); + } catch (\Throwable $error) { + if ($db->inTransaction()) { + $db->rollBack(); + } + if ($this->logger !== null) { + $this->logger->error( + 'Issue Tracker batch edit failed.', + ['exception' => $error] + ); + } + return new \LORIS\Http\Response\JSON\InternalServerError(); + } + + foreach ($notificationQueue as $notification) { + try { + $this->_notifyIssueChange( + $notification['issueID'], + $notification['oldAssignee'], + $notification['newAssignee'], + $user + ); + } catch (\Throwable $error) { + if ($this->logger !== null) { + $this->logger->error( + 'Issue Tracker batch edit notification failed.', + [ + 'exception' => $error, + 'issueID' => $notification['issueID'], + ] + ); + } + } + } + + return new \LORIS\Http\Response\JsonResponse( + [ + 'selected' => count($issueIDs), + 'updated' => $updatedCount, + ] + ); + } + + /** + * Return whether the user may close the selected issue. + * + * @param array $issue The selected issue row. + * @param \User $user The user making the request. + * + * @phpstan-param array{centerID: int|string|null, reporter: string} $issue + * + * @return bool Whether the issue may be closed. + */ + private function _canCloseIssue(array $issue, \User $user) : bool + { + if ($user->hasPermission('issue_tracker_close_all_issue')) { + return true; + } + + if ($user->hasPermission('issue_tracker_close_site_issue') + && $issue['centerID'] !== null + && $user->hasCenter(new \CenterID((string)$issue['centerID'])) + ) { + return true; + } + + return $user->hasPermission('issue_tracker_own_issue') + && $issue['reporter'] === $user->getUsername(); + } + + /** + * Get users configured to receive all Issue Tracker change notifications. + * + * @return string[] User IDs. + */ + private function _getGlobalWatchers() : array + { + return array_map( + 'strval', + $this->loris->getDatabaseConnection()->pselectCol( + "SELECT u.UserID + FROM users_notifications_rel unr + JOIN users u ON unr.user_id=u.ID + WHERE unr.module_id=( + SELECT id FROM notification_modules + WHERE module_name='issue_tracker' + AND operation_type='create/edit' + )", + [] + ) + ); + } + + /** + * Email the assignee and watchers after an issue changes. + * + * @param int $issueID The changed issue ID. + * @param ?string $oldAssignee The assignee before the change. + * @param ?string $newAssignee The assignee after the change. + * @param \User $user The user making the request. + * + * @return void + */ + private function _notifyIssueChange( + int $issueID, + ?string $oldAssignee, + ?string $newAssignee, + \User $user + ) : void { + $db = $this->loris->getDatabaseConnection(); + $issue = $db->pselectRow( + 'SELECT title, priority, status FROM issues WHERE issueID=:issueID', + ['issueID' => $issueID] + ); + $messageData = [ + 'study' => \NDB_Factory::singleton()->config() + ->getSetting('title'), + 'url' => \NDB_Factory::singleton()->settings()->getBaseURL() + . '/issue_tracker/issue/' . $issueID, + 'issueID' => $issueID, + 'currentUser' => $user->getUsername(), + 'title' => $issue['title'] ?? null, + 'priority' => $issue['priority'] ?? null, + 'status' => $issue['status'] ?? null, + 'comment' => null, + ]; + + if ($newAssignee !== null + && $newAssignee !== $user->getUsername() + ) { + /** + * Assignee contact information. + * + * @var ?array{Email: string, First_name: string} $assignee + */ + $assignee = $db->pselectRow( + 'SELECT Email, First_name FROM users WHERE UserID=:userID', + ['userID' => $newAssignee] + ); + if ($assignee !== null) { + $messageData['firstname'] = $assignee['First_name']; + \Email::send( + $assignee['Email'], + $oldAssignee === $newAssignee + ? 'issue_assigned_modified.tpl' + : 'issue_assigned.tpl', + $messageData + ); + } + } + + /** + * Watcher contact information. + * + * @var iterable $watchers + */ + $watchers = $db->pselect( + "SELECT DISTINCT u.Email, u.First_name + FROM users u + JOIN issues_watching iw ON iw.userID=u.UserID + WHERE iw.issueID=:issueID + AND u.UserID<>:currentUser + AND (:hasAssignee=0 OR u.UserID<>:assignee)", + [ + 'issueID' => $issueID, + 'currentUser' => $user->getUsername(), + 'hasAssignee' => $newAssignee === null ? 0 : 1, + 'assignee' => $newAssignee ?? '', + ] + ); + foreach ($watchers as $watcher) { + $messageData['firstname'] = $watcher['First_name']; + \Email::send( + $watcher['Email'], + 'issue_change.tpl', + $messageData + ); + } + } + + /** + * Return true when the user can batch edit issues. + * + * @param \User $user The user whose access is being checked. + * + * @return bool Whether access is granted. + */ + #[\Override] + public function isAccessibleBy(\User $user) : bool + { + return $user->hasPermission('issue_tracker_all_issue'); + } + + /** + * Return the valid HTTP methods for this endpoint. + * + * @return string[] Valid methods. + */ + protected function allowedMethods() : array + { + return ['POST']; + } +} diff --git a/modules/issue_tracker/php/edit.class.inc b/modules/issue_tracker/php/edit.class.inc index fcb31f611c..c43de0fd0e 100644 --- a/modules/issue_tracker/php/edit.class.inc +++ b/modules/issue_tracker/php/edit.class.inc @@ -1,5 +1,18 @@ + * Alizée Wickenheiser + * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 + * @link https://www.github.com/aces/Loris/ + */ + namespace LORIS\issue_tracker; use LORIS\issue_tracker\Provisioners\AttachmentProvisioner; @@ -647,6 +660,13 @@ class Edit extends \NDB_Page implements ETagCalculator ); $comment['newValue'] = $visitLabel; $comment['fieldChanged'] = 'Visit Label'; + } else if ($comment['fieldChanged'] === 'assignee' + && $comment['newValue'] === '' + ) { + $comment['newValue'] = dgettext( + 'issue_tracker', + 'Unassigned' + ); } else if ($comment['fieldChanged'] === 'assignee' || $comment['fieldChanged'] === 'lastUpdatedBy' ) { diff --git a/modules/issue_tracker/test/issue_tracker_test_plan.md b/modules/issue_tracker/test/issue_tracker_test_plan.md index f8ed4e3e79..4c0f07acde 100644 --- a/modules/issue_tracker/test/issue_tracker_test_plan.md +++ b/modules/issue_tracker/test/issue_tracker_test_plan.md @@ -44,6 +44,17 @@ 11. Test if user assigned to issue cannot delete attachments of issue owner. 12. Test that emails are sent to users that are watching the issue. +## Issue Tracker Batch Edit [Manual Testing] +1. Confirm that the Batch Edit tab is only available with the `Issue Tracker: View/Edit/Comment Issues - All Sites` permission. +2. Filter the issue cards and confirm that Select all filtered issues selects only issues matching the current filters. +3. Select issues across multiple pages and update status, priority, category, and assignee together. +4. Confirm that fields set to No change retain their original values. +5. Set category to Uncategorized and assignee to Unassigned and confirm both values are cleared. +6. Confirm that each changed field is added to the issue history, a cleared assignee is displayed as Unassigned, and the editor is recorded as Last Updated By. +7. Confirm that a newly assigned user is added as a watcher. +8. Confirm that Closed and Rejected are unavailable without an Issue Tracker close permission and that the endpoint rejects a selection containing any issue the user cannot close. +9. Submit a mix of changed and unchanged issues and confirm the success count includes only changed issues. + ## Permissions [Automation Testing] 1. Remove `Issue Tracker: View/Edit/Comment Issues - All Sites` permission. 2. Remove `Issue Tracker: View/Edit/Comment Issues - Own Sites` permission. From 9545df2b7a1ceb8aacb6636e6872324b01c5355c Mon Sep 17 00:00:00 2001 From: Montek Date: Sun, 12 Jul 2026 21:57:31 -0400 Subject: [PATCH 2/4] Move issue batch retrieval into BatchEdit endpoint --- .../jsx/IssueTrackerBatchMode.js | 2 +- modules/issue_tracker/php/batchedit.class.inc | 267 +++++++++++++++++- modules/issue_tracker/php/edit.class.inc | 229 --------------- .../issue_tracker/test/Issue_TrackerTest.php | 34 ++- 4 files changed, 292 insertions(+), 240 deletions(-) diff --git a/modules/issue_tracker/jsx/IssueTrackerBatchMode.js b/modules/issue_tracker/jsx/IssueTrackerBatchMode.js index 098c37e21d..9af09b35fe 100644 --- a/modules/issue_tracker/jsx/IssueTrackerBatchMode.js +++ b/modules/issue_tracker/jsx/IssueTrackerBatchMode.js @@ -72,7 +72,7 @@ function IssueTrackerBatchMode({options = {}, canCloseIssues, t}) { async function fetchIssues() { try { const response = await fetch( - `${loris.BaseURL}/issue_tracker/Edit/?batch=true`, + `${loris.BaseURL}/issue_tracker/BatchEdit/`, { credentials: 'include', // This ensures cookies are sent with the request } diff --git a/modules/issue_tracker/php/batchedit.class.inc b/modules/issue_tracker/php/batchedit.class.inc index 8d13fcf066..cf3154e887 100644 --- a/modules/issue_tracker/php/batchedit.class.inc +++ b/modules/issue_tracker/php/batchedit.class.inc @@ -13,6 +13,7 @@ namespace LORIS\issue_tracker; +use LORIS\Middleware\ETagCalculator; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; @@ -24,7 +25,7 @@ use Psr\Http\Message\ServerRequestInterface; * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 * @link https://www.github.com/aces/Loris/ */ -class BatchEdit extends \NDB_Page +class BatchEdit extends \NDB_Page implements ETagCalculator { private const EDITABLE_FIELDS = [ 'assignee', @@ -65,7 +66,7 @@ class BatchEdit extends \NDB_Page */ public function handle(ServerRequestInterface $request) : ResponseInterface { - if ($request->getMethod() !== 'POST') { + if (!in_array($request->getMethod(), $this->allowedMethods(), true)) { return new \LORIS\Http\Response\JSON\MethodNotAllowed( $this->allowedMethods() ); @@ -80,6 +81,12 @@ class BatchEdit extends \NDB_Page ); } + if ($request->getMethod() === 'GET') { + return new \LORIS\Http\Response\JsonResponse( + $this->_getAllIssues($user) + ); + } + $values = json_decode((string)$request->getBody(), true); if (!is_array($values)) { return new \LORIS\Http\Response\JSON\BadRequest( @@ -92,7 +99,7 @@ class BatchEdit extends \NDB_Page return $issueIDs; } - $updates = $this->_validateUpdates($values['updates'] ?? null, $user); + $updates = $this->_validateUpdates($values['updates'] ?? null); if ($updates instanceof ResponseInterface) { return $updates; } @@ -100,6 +107,233 @@ class BatchEdit extends \NDB_Page return $this->_updateIssues($issueIDs, $updates, $user); } + /** + * Fetch all issues and related data for batch editing. + * + * @param \User $user The current user. + * + * @return array{ + * issues: array>, + * assignees: array, + * otherWatchers: array + * } Contains issues, assignees, and watcher information. + */ + private function _getAllIssues(\User $user) : array + { + $db = $this->loris->getDatabaseConnection(); + $assignees = []; + $otherWatchers = []; + + /** + * Users who can be assigned an issue. + * + * @var array $assigneeRows + */ + $assigneeRows = iterator_to_array( + $db->pselect( + "SELECT UserID + FROM users + WHERE Pending_approval = 'N'", + [] + ) + ); + foreach ($assigneeRows as $assignee) { + if (!empty($assignee['UserID'])) { + $assignees[$assignee['UserID']] + = $this->_formatUserInformation($assignee['UserID']); + } + } + + $currentUsername = $user->getUsername(); + /** + * Active users who can watch an issue. + * + * @var array $potentialWatchers + */ + $potentialWatchers = iterator_to_array( + $db->pselect( + "SELECT UserID + FROM users + WHERE Active = 'Y' AND Pending_approval = 'N'", + [] + ) + ); + foreach ($potentialWatchers as $watcher) { + if (!empty($watcher['UserID']) + && $watcher['UserID'] !== $currentUsername + ) { + $otherWatchers[$watcher['UserID']] + = $this->_formatUserInformation($watcher['UserID']); + } + } + + /** + * Issues displayed in batch mode. + * + * @var array> $issues + */ + $issues = iterator_to_array( + $db->pselect( + "SELECT i.*, c.PSCID, s.Visit_label AS visitLabel + FROM issues AS i + LEFT JOIN candidate c ON i.CandidateID = c.ID + LEFT JOIN session s ON i.sessionID = s.ID + ORDER BY i.issueID DESC", + [] + ) + ); + if (empty($issues)) { + return [ + 'issues' => [], + 'assignees' => $assignees, + 'otherWatchers' => $otherWatchers, + ]; + } + + $watcherParams = []; + $watcherPlaceholders = []; + foreach (array_column($issues, 'issueID') as $index => $issueID) { + $param = "issueID$index"; + $watcherPlaceholders[] = ":$param"; + $watcherParams[$param] = $issueID; + } + $watcherPlaceholderList = implode(',', $watcherPlaceholders); + /** + * Watchers grouped by issue. + * + * @var array + * $watcherRows + */ + $watcherRows = iterator_to_array( + $db->pselect( + "SELECT issueID, userID + FROM issues_watching + WHERE issueID IN ($watcherPlaceholderList)", + $watcherParams + ) + ); + + $watchersByIssue = []; + foreach ($watcherRows as $watcher) { + if (!empty($watcher['userID'])) { + $issueID = (int)$watcher['issueID']; + $watchersByIssue[$issueID][] = $watcher['userID']; + } + } + + foreach ($issues as &$issue) { + $issueID = (int)$issue['issueID']; + $issueWatchers = $watchersByIssue[$issueID] ?? []; + $issue['watching'] = in_array( + $currentUsername, + $issueWatchers, + true + ) ? 'Yes' : 'No'; + + $assigneeUserID = $issue['assignee'] ?? null; + if (!empty($assigneeUserID)) { + $issueWatchers = array_values( + array_filter( + $issueWatchers, + fn($watcherUserID) => $watcherUserID !== $assigneeUserID + ) + ); + } + $issue['othersWatching'] = $issueWatchers; + + $issue['reporter'] = !empty($issue['reporter']) + ? $this->_formatUserInformation($issue['reporter']) + : null; + $issue['lastUpdatedBy'] = !empty($issue['lastUpdatedBy']) + ? $this->_formatUserInformation($issue['lastUpdatedBy']) + : null; + $issue['topComments'] = $this->_getTopComments($issueID); + unset($issue['CandidateID']); + } + unset($issue); + + return [ + 'issues' => $issues, + 'assignees' => $assignees, + 'otherWatchers' => $otherWatchers, + ]; + } + + /** + * Fetch the three most recent comments for an issue. + * + * @param int $issueID The issue ID. + * + * @return array The recent comments. + */ + private function _getTopComments(int $issueID) : array + { + /** + * Comment rows returned by the database. + * + * @var iterable $comments + */ + $comments = $this->loris->getDatabaseConnection()->pselect( + "SELECT issueComment, dateAdded, addedBy + FROM issues_comments + WHERE issueID = :issueID + ORDER BY dateAdded DESC + LIMIT 3", + ['issueID' => $issueID] + ); + + $topComments = []; + foreach ($comments as $comment) { + $topComments[] = [ + 'issueComment' => $comment['issueComment'], + 'dateAdded' => $comment['dateAdded'], + 'addedBy' => $this->_formatUserInformation( + $comment['addedBy'] + ), + ]; + } + + return $topComments; + } + + /** + * Format a user ID for display in batch-mode issue cards. + * + * @param ?string $userID The user ID to format. + * + * @return string The formatted user information. + */ + private function _formatUserInformation(?string $userID) : string + { + if ($userID === null) { + return 'AnonymousUser'; + } + + $realName = $this->loris->getDatabaseConnection()->pselectOne( + 'SELECT Real_name FROM users WHERE userid = :userID', + ['userID' => $userID] + ); + if ($realName === null) { + return 'AnonymousUser'; + } + + return (string)$realName . " ($userID)"; + } + /** * Validate and normalize the selected issue IDs. * @@ -130,15 +364,12 @@ class BatchEdit extends \NDB_Page * Validate and normalize the fields to update. * * @param mixed $updates The submitted field updates. - * @param \User $user The user making the request. * * @return array|ResponseInterface Valid updates or an error * response. */ - private function _validateUpdates( - mixed $updates, - \User $user - ) : array|ResponseInterface { + private function _validateUpdates(mixed $updates) : array|ResponseInterface + { if (!is_array($updates) || empty($updates)) { return new \LORIS\Http\Response\JSON\BadRequest( 'At least one field must be updated.' @@ -556,6 +787,24 @@ class BatchEdit extends \NDB_Page return $user->hasPermission('issue_tracker_all_issue'); } + /** + * Calculate an ETag for a batch-edit data request. + * + * @param ServerRequestInterface $request The incoming PSR7 request. + * + * @return string The value to use for the ETag header. + */ + public function ETag(ServerRequestInterface $request) : string + { + if ($request->getMethod() !== 'GET') { + return ''; + } + + return md5( + (string)json_encode((string)$this->handle($request)->getBody()) + ); + } + /** * Return the valid HTTP methods for this endpoint. * @@ -563,6 +812,6 @@ class BatchEdit extends \NDB_Page */ protected function allowedMethods() : array { - return ['POST']; + return ['GET', 'POST']; } } diff --git a/modules/issue_tracker/php/edit.class.inc b/modules/issue_tracker/php/edit.class.inc index c43de0fd0e..65f54c1db9 100644 --- a/modules/issue_tracker/php/edit.class.inc +++ b/modules/issue_tracker/php/edit.class.inc @@ -71,15 +71,6 @@ class Edit extends \NDB_Page implements ETagCalculator $user = $request->getAttribute('user'); $db = $this->loris->getDatabaseConnection(); - // Check if batch mode is enabled - $batch_mode = filter_input(INPUT_GET, 'batch', FILTER_VALIDATE_BOOLEAN); - - if ($batch_mode) { - // Fetch all issues - $allIssues = $this->_getAllIssues($user); - return new \LORIS\Http\Response\JsonResponse($allIssues); - } - // get field options $sites = Issue_Tracker::getSites(false, true); @@ -319,226 +310,6 @@ class Edit extends \NDB_Page implements ETagCalculator ); } - /** - * Fetches all issues including related data and top comments. - * - * @param \User $user The current user. - * - * @return array Contains issues, assignees, and other watchers information. - */ - private function _getAllIssues(\User $user): array - { - $db = $this->loris->getDatabaseConnection(); - $assignees = []; - $otherWatchers = []; - $centerIDsArray = []; - $dccID = null; - $centerPlaceholders = ''; - - // Initialize variables based on permissions - if (!$user->hasPermission('issue_tracker_all_issue')) { - $centerIDsArray = array_map( - fn($centerID) => (int)$centerID->__toString(), - $user->getCenterIDs() - ); - - if (empty($centerIDsArray)) { - return [ - 'issues' => [], - 'assignees' => [], - 'otherWatchers' => [], - ]; - } - - // Fetch DCC ID - $dccRow = $db->pselectOne( - "SELECT CenterID FROM psc WHERE Name = :name", - ['name' => 'DCC'] - ); - $dccID = $dccRow ? (int)$dccRow: null; - - // Prepare placeholders - $centerPlaceholders = implode( - ',', - array_fill(0, count($centerIDsArray), '?') - ); - } - - // Prepare assignee query - if ($user->hasPermission('issue_tracker_all_issue')) { - $assigneeQuery = " - SELECT Real_name, UserID, Active - FROM users - WHERE Pending_approval = 'N' - "; - $assigneeParams = []; - } else { - $assigneeQuery = " - SELECT DISTINCT u.Real_name, u.UserID, u.Active - FROM users u - INNER JOIN user_psc_rel upr ON upr.UserID = u.ID - WHERE (upr.CenterID IN ($centerPlaceholders) OR upr.CenterID = ?) - AND u.Pending_approval = 'N' - "; - $assigneeParams = array_merge($centerIDsArray, [$dccID]); - } - - // Fetch assignees - $assigneeRows = iterator_to_array( - $db->pselect($assigneeQuery, $assigneeParams) - ); - - foreach ($assigneeRows as $a_row) { - if (!empty($a_row['UserID'])) { - $assignees[$a_row['UserID']] = $this->formatUserInformation( - $a_row['UserID'] - ); - } - } - - // Fetch potential watchers - $currentUsername = $user->getUsername(); - - $potentialWatchers = iterator_to_array( - $db->pselect( - "SELECT Real_name, UserID - FROM users - WHERE Active = 'Y' AND Pending_approval = 'N'", - [] - ) - ); - - foreach ($potentialWatchers as $w_row) { - if (!empty($w_row['UserID']) && $w_row['UserID'] != $currentUsername) { - $otherWatchers[$w_row['UserID']] = $this->formatUserInformation( - $w_row['UserID'] - ); - } - } - - // Fetch issues - $baseQuery = " - SELECT i.*, c.PSCID, s.Visit_label AS visitLabel - FROM issues AS i - LEFT JOIN candidate c ON i.CandidateID = c.ID - LEFT JOIN session s ON i.sessionID = s.ID - "; - - $issueParams = []; - if (!$user->hasPermission('issue_tracker_all_issue')) { - $baseQuery .= " WHERE i.centerID IN ($centerPlaceholders)"; - $issueParams = $centerIDsArray; - } - - $baseQuery .= " ORDER BY i.issueID DESC"; - - $issues = iterator_to_array($db->pselect($baseQuery, $issueParams)); - - if (empty($issues)) { - return [ - 'issues' => [], - 'assignees' => $assignees, - 'otherWatchers' => $otherWatchers, - ]; - } - - // Get all issue IDs - $issueIDs = array_column($issues, 'issueID'); - - // Fetch watchers for these issues - $watchingPlaceholders = implode(',', array_fill(0, count($issueIDs), '?')); - $watchersRows = iterator_to_array( - $db->pselect( - "SELECT issueID, userID - FROM issues_watching - WHERE issueID IN ($watchingPlaceholders)", - $issueIDs - ) - ); - - // Organize watchers by issue - $othersWatchingByIssue = []; - foreach ($watchersRows as $row) { - if (!empty($row['userID'])) { - $issueID = (int)$row['issueID']; - $othersWatchingByIssue[$issueID][] = $row['userID']; - } - } - - // Format the issues data - foreach ($issues as &$issue) { - $issueID = (int)$issue['issueID']; - - // Set watching status - $isWatching = in_array( - $currentUsername, - $othersWatchingByIssue[$issueID] ?? [] - ); - $issue['watching'] = $isWatching ? 'Yes' : 'No'; - - $assigneeUserID = $issue['assignee'] ?? null; - - // Exclude assignee from othersWatching - if (!empty($assigneeUserID) && isset($othersWatchingByIssue[$issueID])) { - $filteredWatchers = array_filter( - $othersWatchingByIssue[$issueID], - fn($watcherUserID) => $watcherUserID !== $assigneeUserID - ); - $othersWatchingByIssue[$issueID] = array_values($filteredWatchers); - } - - $issue['othersWatching'] = $othersWatchingByIssue[$issueID] ?? []; - - $issue['reporter'] = !empty($issue['reporter']) - ? $this->formatUserInformation($issue['reporter']) - : null; - $issue['lastUpdatedBy'] = !empty($issue['lastUpdatedBy']) - ? $this->formatUserInformation($issue['lastUpdatedBy']) - : null; - - $issue['topComments'] = $this->getTopComments($issueID); - unset($issue['CandidateID']); - } - - return [ - 'issues' => $issues, - 'assignees' => $assignees, - 'otherWatchers' => $otherWatchers, - ]; - } - - /** - * Fetches the top 3 most recent comments for an issue. - * - * @param int $issueID The ID of the issue. - * - * @return array Top 3 comments with comment text, date, and author. - */ - function getTopComments(int $issueID): array - { - $db = $this->loris->getDatabaseConnection(); - - $comments = $db->pselect( - "SELECT issueComment, dateAdded, addedBy - FROM issues_comments - WHERE issueID = :issueID - ORDER BY dateAdded DESC - LIMIT 3", - ['issueID' => $issueID] - ); - - $topComments = []; - foreach ($comments as $comment) { - $topComments[] = [ - 'issueComment' => $comment['issueComment'], - 'dateAdded' => $comment['dateAdded'], - 'addedBy' => $this->formatUserInformation($comment['addedBy']), - ]; - } - - return $topComments; - } - /** * If issueID is passed retrieves issue data from database, * otherwise return empty issue data object diff --git a/modules/issue_tracker/test/Issue_TrackerTest.php b/modules/issue_tracker/test/Issue_TrackerTest.php index 07e0f6f873..04be247f8f 100644 --- a/modules/issue_tracker/test/Issue_TrackerTest.php +++ b/modules/issue_tracker/test/Issue_TrackerTest.php @@ -122,6 +122,39 @@ function testIssueTrackerDoespageLoadWithPermission() $this->resetPermissions(); } + /** + * Tests that the Batch Edit endpoint returns the issues used by batch mode. + * + * @return void + */ + function testBatchEditEndpointReturnsIssues() + { + $this->setupPermissions(["issue_tracker_all_issue"]); + $this->webDriver->get($this->url . "/issue_tracker/BatchEdit/"); + $bodyText = $this->webDriver->getPageSource(); + $this->assertStringContainsString('"issues"', $bodyText); + $this->assertStringContainsString('Test Issue', $bodyText); + $this->resetPermissions(); + } + + /** + * Tests that Batch Edit rejects users without all-issue access. + * + * @return void + */ + function testBatchEditEndpointRejectsSitePermission() + { + $this->setupPermissions(["issue_tracker_site_issue"]); + $this->webDriver->get($this->url . "/issue_tracker/BatchEdit/"); + $bodyText = $this->webDriver->getPageSource(); + $this->assertStringContainsString( + 'You do not have access to this page.', + $bodyText + ); + $this->assertStringNotContainsString('"issues"', $bodyText); + $this->resetPermissions(); + } + /** * Tests that Issue Tracker's filters * @@ -173,4 +206,3 @@ function testClearFormIssueTracker() $this->assertStringNotContainsString("TestTestTest", $bodyText); } } - From f59a72fc846908ed27d00ad281409db81cf2dab5 Mon Sep 17 00:00:00 2001 From: Montek Date: Wed, 15 Jul 2026 09:46:58 -0400 Subject: [PATCH 3/4] [issue_tracker] address batch edit UI review feedback --- .../css/issue_tracker_batchmode.css | 87 +++++++++-- modules/issue_tracker/jsx/IssueCard.js | 23 ++- .../jsx/IssueTrackerBatchMode.js | 145 ++++++++++++------ .../issue_tracker/locale/issue_tracker.pot | 12 ++ .../test/issue_tracker_test_plan.md | 5 + 5 files changed, 206 insertions(+), 66 deletions(-) diff --git a/modules/issue_tracker/css/issue_tracker_batchmode.css b/modules/issue_tracker/css/issue_tracker_batchmode.css index e74e0e0ef3..d58da08660 100644 --- a/modules/issue_tracker/css/issue_tracker_batchmode.css +++ b/modules/issue_tracker/css/issue_tracker_batchmode.css @@ -3,6 +3,7 @@ flex-direction: column; max-width: 1000px; margin: 0 auto; + padding-bottom: 140px; } .filter-tabs { @@ -220,15 +221,32 @@ padding: 10px; } -.batch-selection-controls label { +.batch-select-all-control { display: flex; align-items: center; margin: 0; } +.batch-select-all-control .checkbox { + margin: 0 8px 0 0; +} + +.batch-section-title { + flex-basis: 100%; + margin: 0; + padding-bottom: 8px; + border-bottom: 1px solid #ddd; + font-size: 14px; + font-weight: bold; +} + +.batch-changes-title { + margin: 10px 10px 0; +} + .batch-edit-controls { display: grid; - grid-template-columns: repeat(4, minmax(150px, 1fr)) auto; + grid-template-columns: repeat(4, minmax(150px, 1fr)); align-items: end; gap: 15px; padding: 10px; @@ -246,28 +264,58 @@ white-space: nowrap; } -.issue-selection-card { - border: 2px solid transparent; - border-radius: 8px; +.batch-actions-panel { + position: fixed; + right: 20px; + bottom: 20px; + z-index: 1030; + width: 360px; + padding: 12px; + border: 1px solid #9aafc4; + border-radius: 4px; + background-color: #fff; + box-shadow: 0 3px 12px rgba(0, 0, 0, 0.25); } -.issue-selection-card.selected { +.batch-actions-header, +.batch-action-buttons { + display: flex; + align-items: center; + justify-content: space-between; + gap: 10px; +} + +.batch-scroll-button { + min-width: 34px; + font-size: 18px; + line-height: 1; +} + +.batch-actions-summary { + margin: 10px 0; + padding: 8px 0; + border-top: 1px solid #ddd; + border-bottom: 1px solid #ddd; +} + +.batch-action-buttons .btn { + flex: 1; +} + +.issue-card.batch-selected { border-color: #074785; } -.issue-selection-control { - display: flex; - align-items: center; - margin: 0; - padding: 8px 12px; - background-color: #f8f8f8; - border: 1px solid #ddd; - border-bottom: 0; +.issue-card.batch-selected .issue-header { + margin: -20px -20px 15px; + padding: 12px 20px; border-radius: 8px 8px 0 0; + background-color: #e4edf5; } -.issue-selection-card .issue-card { - border-radius: 0 0 8px 8px; +.issue-selection-checkbox { + flex: 0 0 auto; + margin: 0 8px 0 0; } @media (max-width: 900px) { @@ -280,4 +328,11 @@ .batch-edit-controls { grid-template-columns: 1fr; } + + .batch-actions-panel { + right: 10px; + bottom: 10px; + left: 10px; + width: auto; + } } diff --git a/modules/issue_tracker/jsx/IssueCard.js b/modules/issue_tracker/jsx/IssueCard.js index 9bf8ae50fc..7747c1fb19 100644 --- a/modules/issue_tracker/jsx/IssueCard.js +++ b/modules/issue_tracker/jsx/IssueCard.js @@ -1,4 +1,4 @@ -import React, {useState} from 'react'; +import React, {useEffect, useState} from 'react'; import PropTypes from 'prop-types'; import swal from 'sweetalert2'; import Modal from 'jsx/Modal'; @@ -18,6 +18,8 @@ const IssueCard = React.memo(function IssueCard(props) { sites, assignees, otherWatchers, + isSelected, + onToggleSelection, } = props; const [isEditing, setIsEditing] = useState(false); @@ -32,6 +34,11 @@ const IssueCard = React.memo(function IssueCard(props) { const [newAssignee, setNewAssignee] = useState(issue.assignee || ''); const [newWatchers, setNewWatchers] = useState(issue.othersWatching || []); + useEffect(() => { + setEditedIssue({...issue}); + setTempEditedIssue({...issue}); + }, [issue]); + const handleInputChange = (field, value) => { setTempEditedIssue((prev) => ({ ...prev, @@ -208,7 +215,7 @@ const IssueCard = React.memo(function IssueCard(props) { const description = editedIssue.description || ''; return ( -
+
+ {onToggleSelection && ( + + )}

#{issue.issueID} {isEditing ? ( @@ -572,6 +589,8 @@ IssueCard.propTypes = { sites: PropTypes.object.isRequired, assignees: PropTypes.object.isRequired, otherWatchers: PropTypes.object.isRequired, + isSelected: PropTypes.bool, + onToggleSelection: PropTypes.func, t: PropTypes.func, }; diff --git a/modules/issue_tracker/jsx/IssueTrackerBatchMode.js b/modules/issue_tracker/jsx/IssueTrackerBatchMode.js index 9af09b35fe..a0d94d16fd 100644 --- a/modules/issue_tracker/jsx/IssueTrackerBatchMode.js +++ b/modules/issue_tracker/jsx/IssueTrackerBatchMode.js @@ -32,6 +32,7 @@ function IssueTrackerBatchMode({options = {}, canCloseIssues, t}) { const [assignees, setAssignees] = useState({}); const [otherWatchers, setOtherWatchers] = useState({}); const [selectedIssueIDs, setSelectedIssueIDs] = useState([]); + const [selectAllFiltered, setSelectAllFiltered] = useState(false); const [batchUpdates, setBatchUpdates] = useState({ status: NO_CHANGE, priority: NO_CHANGE, @@ -66,6 +67,12 @@ function IssueTrackerBatchMode({options = {}, canCloseIssues, t}) { issues, ]); + useEffect(() => { + if (selectAllFiltered) { + setSelectedIssueIDs(filteredIssues.map((issue) => issue.issueID)); + } + }, [filteredIssues, selectAllFiltered]); + /** * Fetches issues from the server */ @@ -75,6 +82,7 @@ function IssueTrackerBatchMode({options = {}, canCloseIssues, t}) { `${loris.BaseURL}/issue_tracker/BatchEdit/`, { credentials: 'include', // This ensures cookies are sent with the request + cache: 'no-store', } ); if (!response.ok) { @@ -150,6 +158,9 @@ function IssueTrackerBatchMode({options = {}, canCloseIssues, t}) { setSelectedStatuses([]); setSelectedSites([]); setSelectedAssignees([]); + if (!selectAllFiltered) { + setSelectedIssueIDs([]); + } } /** @@ -165,6 +176,7 @@ function IssueTrackerBatchMode({options = {}, canCloseIssues, t}) { * @param {number} issueID - The issue ID to toggle */ function toggleIssueSelection(issueID) { + setSelectAllFiltered(false); setSelectedIssueIDs((current) => current.includes(issueID) ? current.filter((id) => id !== issueID) : [...current, issueID] @@ -179,12 +191,31 @@ function IssueTrackerBatchMode({options = {}, canCloseIssues, t}) { const allFilteredSelected = filteredIssueIDs.length > 0 && filteredIssueIDs.every((issueID) => selectedIssueIDs.includes(issueID)); + setSelectAllFiltered(!allFilteredSelected); setSelectedIssueIDs((current) => allFilteredSelected ? current.filter((issueID) => !filteredIssueIDs.includes(issueID)) : - [...new Set([...current, ...filteredIssueIDs])] + filteredIssueIDs ); } + /** + * Clears the batch selection and exits select-all-filtered mode. + */ + function clearSelection() { + setSelectAllFiltered(false); + setSelectedIssueIDs([]); + } + + /** + * Returns to the batch configuration panel. + */ + function scrollToBatchConfiguration() { + document.getElementById('batch-edit-panel')?.scrollIntoView({ + behavior: 'smooth', + block: 'start', + }); + } + /** * Updates one field in the pending batch changes. * @@ -242,7 +273,7 @@ function IssueTrackerBatchMode({options = {}, canCloseIssues, t}) { throw new Error(data.error || 'Network response was not ok'); } - setSelectedIssueIDs([]); + clearSelection(); setBatchUpdates({ status: NO_CHANGE, priority: NO_CHANGE, @@ -255,6 +286,8 @@ function IssueTrackerBatchMode({options = {}, canCloseIssues, t}) { text: t('{{count}} issue updated successfully', { ns: 'issue_tracker', count: data.updated, + defaultValue_one: '{{count}} issue updated successfully', + defaultValue_other: '{{count}} issues updated successfully', }), }); } catch (error) { @@ -306,6 +339,13 @@ function IssueTrackerBatchMode({options = {}, canCloseIssues, t}) { filteredIssueIDs.every((issueID) => selectedIssueIDs.includes(issueID)); const hasBatchUpdates = Object.values(batchUpdates) .some((value) => value !== NO_CHANGE); + const hasSelectedIssues = selectedIssueIDs.length > 0; + const selectedIssueCount = t('{{count}} issue selected', { + ns: 'issue_tracker', + count: selectedIssueIDs.length, + defaultValue_one: '{{count}} issue selected', + defaultValue_other: '{{count}} issues selected', + }); const tabList = [ { @@ -495,7 +535,10 @@ function IssueTrackerBatchMode({options = {}, canCloseIssues, t}) { className="panel-default" >
-
+

+ {t('Changes to apply', {ns: 'issue_tracker'})} +

+
+ +
@@ -651,31 +712,19 @@ function IssueTrackerBatchMode({options = {}, canCloseIssues, t}) {
{paginatedIssues.length > 0 ? ( paginatedIssues.map((issue) => ( -
- - -
+ issue={issue} + assignees={assignees} + otherWatchers={otherWatchers} + onUpdate={handleIssueUpdate} + statuses={statuses} + priorities={priorities} + categories={categories} + sites={sites} + isSelected={selectedIssueIDs.includes(issue.issueID)} + onToggleSelection={() => toggleIssueSelection(issue.issueID)} + /> )) ) : (
diff --git a/modules/issue_tracker/locale/issue_tracker.pot b/modules/issue_tracker/locale/issue_tracker.pot index ae528e4c7d..b9d7d68d15 100644 --- a/modules/issue_tracker/locale/issue_tracker.pot +++ b/modules/issue_tracker/locale/issue_tracker.pot @@ -103,6 +103,18 @@ msgstr "" msgid "Batch changes" msgstr "" +msgid "Batch actions" +msgstr "" + +msgid "Back to batch configuration" +msgstr "" + +msgid "Selection" +msgstr "" + +msgid "Changes to apply" +msgstr "" + msgid "Select all filtered issues" msgstr "" diff --git a/modules/issue_tracker/test/issue_tracker_test_plan.md b/modules/issue_tracker/test/issue_tracker_test_plan.md index 4c0f07acde..ced2ba9aba 100644 --- a/modules/issue_tracker/test/issue_tracker_test_plan.md +++ b/modules/issue_tracker/test/issue_tracker_test_plan.md @@ -54,6 +54,11 @@ 7. Confirm that a newly assigned user is added as a watcher. 8. Confirm that Closed and Rejected are unavailable without an Issue Tracker close permission and that the endpoint rejects a selection containing any issue the user cannot close. 9. Submit a mix of changed and unchanged issues and confirm the success count includes only changed issues. +10. Confirm that the floating Batch actions panel remains available while scrolling and that its up-arrow returns to the Batch changes panel. +11. Confirm that batch fields are disabled with no selected issues and become available after selecting an issue. +12. Enable Select all filtered issues, change and reset the filters, and confirm the selected issue count continues to match the filtered issues. +13. Confirm that each issue checkbox appears beside its issue ID and that selected cards use a highlighted header without a footer band. +14. Apply a batch change to multiple issues and confirm that the plural success message and updated card values appear without refreshing the page. ## Permissions [Automation Testing] 1. Remove `Issue Tracker: View/Edit/Comment Issues - All Sites` permission. From 7d8398e0877513811fc0e08998809789244f7a8b Mon Sep 17 00:00:00 2001 From: Montek Date: Sat, 18 Jul 2026 20:39:47 -0400 Subject: [PATCH 4/4] [issue_tracker] address follow-up batch edit review feedback --- .../css/issue_tracker_batchmode.css | 12 +- .../jsx/IssueTrackerBatchMode.js | 98 +++++++--- .../issue_tracker/jsx/issueTrackerIndex.js | 42 ++-- .../issue_tracker/locale/issue_tracker.pot | 2 +- modules/issue_tracker/php/batchedit.class.inc | 48 ++++- .../issue_tracker/test/Issue_TrackerTest.php | 182 +++++++++++++++++- .../test/issue_tracker_test_plan.md | 6 +- 7 files changed, 334 insertions(+), 56 deletions(-) diff --git a/modules/issue_tracker/css/issue_tracker_batchmode.css b/modules/issue_tracker/css/issue_tracker_batchmode.css index d58da08660..54da3ee39b 100644 --- a/modules/issue_tracker/css/issue_tracker_batchmode.css +++ b/modules/issue_tracker/css/issue_tracker_batchmode.css @@ -213,6 +213,12 @@ margin-right: 10px; } +.batch-collapse-button { + margin-left: auto; + padding: 0 10px; + color: inherit; +} + .batch-selection-controls { display: flex; align-items: center; @@ -246,7 +252,7 @@ .batch-edit-controls { display: grid; - grid-template-columns: repeat(4, minmax(150px, 1fr)); + grid-template-columns: repeat(5, minmax(150px, 1fr)); align-items: end; gap: 15px; padding: 10px; @@ -286,9 +292,7 @@ } .batch-scroll-button { - min-width: 34px; - font-size: 18px; - line-height: 1; + white-space: nowrap; } .batch-actions-summary { diff --git a/modules/issue_tracker/jsx/IssueTrackerBatchMode.js b/modules/issue_tracker/jsx/IssueTrackerBatchMode.js index a0d94d16fd..5fc784852f 100644 --- a/modules/issue_tracker/jsx/IssueTrackerBatchMode.js +++ b/modules/issue_tracker/jsx/IssueTrackerBatchMode.js @@ -10,6 +10,13 @@ import '../css/issue_tracker_batchmode.css'; import {withTranslation} from 'react-i18next'; const NO_CHANGE = '__no_change__'; +const INITIAL_BATCH_UPDATES = { + status: NO_CHANGE, + priority: NO_CHANGE, + category: NO_CHANGE, + centerID: NO_CHANGE, + assignee: NO_CHANGE, +}; /** * IssueTrackerBatchMode component @@ -33,13 +40,10 @@ function IssueTrackerBatchMode({options = {}, canCloseIssues, t}) { const [otherWatchers, setOtherWatchers] = useState({}); const [selectedIssueIDs, setSelectedIssueIDs] = useState([]); const [selectAllFiltered, setSelectAllFiltered] = useState(false); - const [batchUpdates, setBatchUpdates] = useState({ - status: NO_CHANGE, - priority: NO_CHANGE, - category: NO_CHANGE, - assignee: NO_CHANGE, - }); + const [batchUpdates, setBatchUpdates] = useState(INITIAL_BATCH_UPDATES); + const [isBatchChangesCollapsed, setIsBatchChangesCollapsed] = useState(true); const [isSubmitting, setIsSubmitting] = useState(false); + const hasSelectedIssues = selectedIssueIDs.length > 0; // Pagination state const [page, setPage] = useState({ @@ -73,6 +77,12 @@ function IssueTrackerBatchMode({options = {}, canCloseIssues, t}) { } }, [filteredIssues, selectAllFiltered]); + useEffect(() => { + if (hasSelectedIssues) { + setIsBatchChangesCollapsed(false); + } + }, [hasSelectedIssues]); + /** * Fetches issues from the server */ @@ -210,9 +220,12 @@ function IssueTrackerBatchMode({options = {}, canCloseIssues, t}) { * Returns to the batch configuration panel. */ function scrollToBatchConfiguration() { - document.getElementById('batch-edit-panel')?.scrollIntoView({ - behavior: 'smooth', - block: 'start', + setIsBatchChangesCollapsed(false); + requestAnimationFrame(() => { + document.getElementById('batch-edit-panel')?.scrollIntoView({ + behavior: 'smooth', + block: 'start', + }); }); } @@ -273,13 +286,16 @@ function IssueTrackerBatchMode({options = {}, canCloseIssues, t}) { throw new Error(data.error || 'Network response was not ok'); } + if (Number(data.updated) === 0) { + await swal.fire({ + type: 'info', + text: t('No changes were made', {ns: 'issue_tracker'}), + }); + return; + } + clearSelection(); - setBatchUpdates({ - status: NO_CHANGE, - priority: NO_CHANGE, - category: NO_CHANGE, - assignee: NO_CHANGE, - }); + setBatchUpdates(INITIAL_BATCH_UPDATES); await fetchIssues(); await swal.fire({ type: 'success', @@ -339,7 +355,6 @@ function IssueTrackerBatchMode({options = {}, canCloseIssues, t}) { filteredIssueIDs.every((issueID) => selectedIssueIDs.includes(issueID)); const hasBatchUpdates = Object.values(batchUpdates) .some((value) => value !== NO_CHANGE); - const hasSelectedIssues = selectedIssueIDs.length > 0; const selectedIssueCount = t('{{count}} issue selected', { ns: 'issue_tracker', count: selectedIssueIDs.length, @@ -408,6 +423,26 @@ function IssueTrackerBatchMode({options = {}, canCloseIssues, t}) {
); + const batchPanelTitle = ( +
+ {t('Batch changes', {ns: 'issue_tracker'})} + +
+ ); + return (
@@ -614,6 +650,27 @@ function IssueTrackerBatchMode({options = {}, canCloseIssues, t}) { ))} +