[Issue Tracker] Issue Change Notifications (#8885)

Fixed issue where the assignee dropdown did not show the current assignee Emails users who were added as watching When a comment is added, it adds it to the email to everyone watching
aces · Jan 11, 2024 · 03d2092 · 03d2092
1 parent 32ef64d
commit 03d2092
Show file tree

Hide file tree

Showing 5 changed files with 133 additions and 28 deletions.
diff --git a/modules/issue_tracker/jsx/IssueForm.js b/modules/issue_tracker/jsx/IssueForm.js
@@ -26,7 +26,7 @@ class IssueForm extends Component {
     super(props);
 
     this.state = {
-      Data: [],
+      Data: {},
       formData: {},
       submissionResult: null,
       errorMessage: null,

diff --git a/modules/issue_tracker/php/edit.class.inc b/modules/issue_tracker/php/edit.class.inc
@@ -222,6 +222,10 @@ class Edit extends \NDB_Page implements ETagCalculator
             $issueData['attachments']    = $attachments;
             $issueData['whoami']         = $user->getUsername();
             $issueData['othersWatching'] = array_keys($othersWatching);
+            $issueData['assignee']       = $db->pselectOne(
+                "SELECT assignee FROM issues WHERE issueID=:issueID",
+                ['issueID' => $issueID]
+            );
 
             // We need to unescape the string here:
             // React is escaping the string in the template
@@ -370,7 +374,12 @@ class Edit extends \NDB_Page implements ETagCalculator
         $watching = $db->pselect(
             "SELECT Real_name, UserID from users
             WHERE UserID IN 
-            (SELECT UserID FROM issues_watching WHERE issueID=:issueID)",
+              (SELECT userID from issues_watching
+              WHERE issueID=:issueID)
+            AND UserID NOT IN
+              (SELECT assignee FROM issues
+              WHERE issueID=:issueID
+              AND assignee IS NOT NULL)",
             ['issueID' => $issueID]
         );
 
@@ -454,6 +463,11 @@ class Edit extends \NDB_Page implements ETagCalculator
 
         $issueValues['lastUpdatedBy'] = $user->getUsername();
 
+        $assignee = $db->pselectOne(
+            "SELECT assignee FROM issues WHERE issueID=:ID",
+            ['ID' => $issueID]
+        );
+
         $validatedInput = $this->validateInput($validateValues, $user);
         if (!is_array($validatedInput)) { // Error exists.
             return $validatedInput;
@@ -504,18 +518,6 @@ class Edit extends \NDB_Page implements ETagCalculator
             }
         }
 
-        // Adding new assignee to watching
-        if (isset($issueValues['assignee'])) {
-            $nowWatching = [
-                'userID'  => $issueValues['assignee'],
-                'issueID' => $issueID,
-            ];
-            $db->replace('issues_watching', $nowWatching);
-
-            // sending email
-            $this->emailUser($issueID, $issueValues['assignee'], '', $user);
-        }
-
         // Adding others from multiselect to watching table.
         if (isset($values['othersWatching'])) {
 
@@ -542,19 +544,34 @@ class Edit extends \NDB_Page implements ETagCalculator
                     ) {
                         continue;
                     }
-                    $this->emailUser($issueID, '', $usersWatching, $user);
+                    $this->emailUser(
+                        $issueID,
+                        '',
+                        $usersWatching,
+                        $user,
+                        'false',
+                        $values
+                    );
                 }
             }
         }
 
         // Add editor to the watching table unless they don't want to be added.
-        if (isset($values['watching']) && $values['watching'] == 'Yes') {
+        if (isset($values['watching'])
+            && $values['watching'] == 'Yes'
+            && (!isset($issueValues['assignee'])
+            || $issueValues['assignee'] !== $user->getUsername())
+        ) {
             $nowWatching = [
                 'userID'  => $user->getUsername(),
                 'issueID' => $issueID,
             ];
             $db->replace('issues_watching', $nowWatching);
-        } else if (isset($values['watching']) && $values['watching'] == 'No') {
+        } else if (isset($values['watching'])
+            && $values['watching'] == 'No'
+            && (!isset($issueValues['assignee'])
+            || $issueValues['assignee'] !== $user->getUsername())
+        ) {
             $db->delete(
                 'issues_watching',
                 [
@@ -563,6 +580,46 @@ class Edit extends \NDB_Page implements ETagCalculator
                 ]
             );
         }
+        // Adding new assignee to watching
+        if (isset($issueValues['assignee'])
+            && $issueValues['assignee'] !== $assignee
+        ) {
+            $nowWatching = [
+                'userID'  => $issueValues['assignee'],
+                'issueID' => $issueID
+            ];
+            $db->replace('issues_watching', $nowWatching);
+            // sending email
+            $this->emailUser(
+                $issueID,
+                $issueValues['assignee'],
+                isset($usersWatching) ? $usersWatching : '',
+                $user,
+                'false',
+                $values
+            );
+        } else if (isset($issueValues['assignee'])
+            && $issueValues['assignee'] === $assignee
+        ) {
+            // sending email
+            $this->emailUser(
+                $issueID,
+                $issueValues['assignee'],
+                isset($usersWatching) ? $usersWatching : '',
+                $user,
+                'true',
+                $values
+            );
+        } else {
+            $this->emailUser(
+                $issueID,
+                '',
+                isset($usersWatching) ? $usersWatching : '',
+                $user,
+                'false',
+                $values
+            );
+        }
         return new \LORIS\Http\Response\JsonResponse(
             ['issueID' => $issueID]
         );
@@ -575,11 +632,13 @@ class Edit extends \NDB_Page implements ETagCalculator
      * @param string $changed_assignee changed assignee
      * @param string $changed_watcher  changed watcher
      * @param \User  $user             the user requesting the change
+     * @param string $new_assignee_tag boolean of whether it is a new assignee
+     * @param array  $values           the values the user entered in the form
      *
      * @return void
      */
     function emailUser(int $issueID, string $changed_assignee,
-        string $changed_watcher, \User $user
+        string $changed_watcher, \User $user, string $new_assignee_tag, array $values
     ) {
         $db      = $this->loris->getDatabaseConnection();
         $baseurl = \NDB_Factory::singleton()->settings()->getBaseURL();
@@ -596,17 +655,36 @@ class Edit extends \NDB_Page implements ETagCalculator
         $msg_data['issueID'] = $issueID;
         $msg_data['currentUser'] = $user->getUsername();
         $msg_data['title']       = $title;
+        $msg_data['comment']     = $values['comment'];
 
-        if (isset($changed_assignee)) {
+        if (isset($changed_assignee) && $new_assignee_tag == 'false') {
             $issueChangeEmailsAssignee = $db->pselect(
                 "SELECT
-                            u.Email AS Email,
-                            u.First_name AS firstname
-                        FROM
-                            users u
-                        WHERE
-                            u.UserID = :assignee
-                            AND u.UserID != :currentUser",
+                    u.Email AS Email,
+                    u.First_name AS firstname
+                FROM
+                    users u
+                WHERE
+                    u.UserID = :assignee
+                    AND u.UserID != :currentUser",
+                [
+                    'assignee'    => $changed_assignee,
+                    'currentUser' => $user->getUserName(),
+                ]
+            );
+            if (isset($issueChangeEmailsAssignee[0])) {
+                $msg_data['firstname'] = $issueChangeEmailsAssignee[0]['firstname'];
+                \Email::send(
+                    $issueChangeEmailsAssignee[0]['Email'],
+                    'issue_assigned.tpl',
+                    $msg_data
+                );
+            }
+        } else if (isset($changed_assignee) && $new_assignee_tag === 'true') {
+            $issueChangeEmailsAssignee = $db->pselect(
+                "SELECT u.Email as Email, u.First_name as firstname " .
+                "FROM users u WHERE u.UserID=:assignee
+                AND u.UserID<>:currentUser",
                 [
                     'assignee'    => $changed_assignee,
                     'currentUser' => $user->getUserName(),
@@ -618,7 +696,7 @@ class Edit extends \NDB_Page implements ETagCalculator
 
                 \Email::send(
                     $issueChangeEmailsAssignee[0]['Email'],
-                    'issue_assigned.tpl',
+                    'issue_assigned_modified.tpl',
                     $msg_data
                 );
             }
@@ -634,11 +712,13 @@ class Edit extends \NDB_Page implements ETagCalculator
                     WHERE
                         w.issueID = :issueID
                         AND u.UserID = :watcher
+                        AND u.UserID != :assignee
                         AND u.UserID != :currentUser",
             [
                 'issueID'     => $issueID,
                 'watcher'     => $changed_watcher,
                 'currentUser' => $user->getUserName(),
+                'assignee'    => $changed_assignee,
             ]
         );
 
@@ -829,6 +909,10 @@ class Edit extends \NDB_Page implements ETagCalculator
     function updateHistory(array $values, int $issueID, \User $user)
     {
         $db = $this->loris->getDatabaseConnection();
+        $originalAssignee = $db->pselectOne(
+            'SELECT assignee FROM issues where issueID = :issueID',
+            ['issueID' => $issueID]
+        );
         foreach ($values as $key => $value) {
             // centerID is allowed to be NULL
             if (!empty($value) || $key === 'centerID') {
@@ -838,6 +922,9 @@ class Edit extends \NDB_Page implements ETagCalculator
                     'issueID'      => $issueID,
                     'addedBy'      => $user->getUsername(),
                 ];
+                if ($key === 'assignee' && $originalAssignee === $value) {
+                    continue;
+                }
                 $db->unsafeInsert('issues_history', $changedValues);
             }
         }

diff --git a/modules/issue_tracker/test/issue_tracker_test_plan.md b/modules/issue_tracker/test/issue_tracker_test_plan.md
@@ -40,6 +40,7 @@
 10. Test if users assigned to issues can upload attachments.
 11. Test if users can delete their own uploaded attachments.
 12. Test if user assigned to issue cannot delete attachments of issue owner.
+13. Test that emails are sent to users that are watching the issue.
 
 ## Permissions [Automation Testing]
 1. Remove `access_all_profiles` permission.

diff --git a/smarty/templates/email/issue_assigned_modified.tpl b/smarty/templates/email/issue_assigned_modified.tpl
@@ -0,0 +1,14 @@
+Subject: Issue Assigned - # {$issueID}
+{$firstname},
+
+The issue "{$title}" that is assigned to you has been modified.
+
+{if $comment !== "null"}
+    {$currentUser} commented: "{$comment}"
+
+{/if}
+Please see the issue here: {$url}
+
+Thank you,
+
+LORIS Team
diff --git a/smarty/templates/email/issue_change.tpl b/smarty/templates/email/issue_change.tpl
@@ -1,9 +1,12 @@
 Subject: Change to Issue # - {$issueID}
-
 {$firstname},
 
 {$currentUser} has updated an issue "{$title}" you are watching.
 
+{if $comment !== "null"}
+    {$currentUser} commented: "{$comment}"
+{/if}
+
 Please view the changes here: {$url}
 
 Thank you,