Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Issue Tracker] Issue Change Notifications #8885

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion modules/issue_tracker/jsx/IssueForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class IssueForm extends Component {
super(props);

this.state = {
Data: [],
Data: {},
formData: {},
submissionResult: null,
errorMessage: null,
Expand Down
139 changes: 113 additions & 26 deletions modules/issue_tracker/php/edit.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]
);

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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'])) {

Expand All @@ -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',
[
Expand All @@ -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]
);
Expand All @@ -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();
Expand All @@ -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(),
Expand All @@ -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
);
}
Expand All @@ -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,
]
);

Expand Down Expand Up @@ -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') {
Expand All @@ -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);
}
}
Expand Down
1 change: 1 addition & 0 deletions modules/issue_tracker/test/issue_tracker_test_plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
14 changes: 14 additions & 0 deletions smarty/templates/email/issue_assigned_modified.tpl
Original file line number Diff line number Diff line change
@@ -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
5 changes: 4 additions & 1 deletion smarty/templates/email/issue_change.tpl
Original file line number Diff line number Diff line change
@@ -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,
Expand Down