Skip to content

Commit

Permalink
Add recipient exclusions for notifications
Browse files Browse the repository at this point in the history
  • Loading branch information
cconard96 authored Oct 22, 2024
1 parent 6271d05 commit ff21253
Show file tree
Hide file tree
Showing 7 changed files with 198 additions and 8 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ The present file will list all changes made to the project; according to the
- The debug tab that was present, for some items, when the debug mode was active, no longer exists. The corresponding features have been either moved, either removed.
- `Group` and `Group in charge` fields for assets may now contain multiple groups.
- "If software are no longer used" transfer option is now taken into account rather than always preserving.
- Notifications can now specify exclusions for recipients.

### Deprecated
- Survey URL tags `TICKETCATEGORY_ID` and `TICKETCATEGORY_NAME` are deprecated and replaced by `ITILCATEGORY_ID` and `ITILCATEGORY_NAME` respectively.
Expand All @@ -94,6 +95,7 @@ The present file will list all changes made to the project; according to the
- `phpCAS` library is now bundled in GLPI, to prevent version compatibility issues.
- `Glpi\DBAL\QueryFunction` class with multiple static methods for building SQL query function strings in an abstract way.
- `fetchSessionMessages()` global JS function to display new session messages as toast notifications without requiring a page reload.
- `is_exclusion` column added to `glpi_notificationtargets` table.

#### Changes
- Many methods have their signature changed to specify both their return type and the types of their parameters.
Expand Down
3 changes: 3 additions & 0 deletions install/migrations/update_10.0.x_to_11.0.0/notifications.php
Original file line number Diff line number Diff line change
Expand Up @@ -317,3 +317,6 @@
"item_trigger"
);
/** /Add handling of source item of attached documents in notification */

$migration->addField('glpi_notificationtargets', 'is_exclusion', 'boolean');
$migration->addKey('glpi_notificationtargets', ['is_exclusion'], 'is_exclusion');
4 changes: 3 additions & 1 deletion install/mysql/glpi-empty.sql
Original file line number Diff line number Diff line change
Expand Up @@ -5003,9 +5003,11 @@ CREATE TABLE `glpi_notificationtargets` (
`items_id` int unsigned NOT NULL DEFAULT '0',
`type` int NOT NULL DEFAULT '0',
`notifications_id` int unsigned NOT NULL DEFAULT '0',
`is_exclusion` tinyint NOT NULL DEFAULT '0',
PRIMARY KEY (`id`),
KEY `items` (`type`,`items_id`),
KEY `notifications_id` (`notifications_id`)
KEY `notifications_id` (`notifications_id`),
KEY `is_exclusion` (`is_exclusion`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci ROW_FORMAT=DYNAMIC;


Expand Down
63 changes: 63 additions & 0 deletions phpunit/functional/NotificationTargetTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -575,4 +575,67 @@ public function testGetMessageIdForEvent(?string $itemtype, ?int $items_id, ?str
$messageid = $instance->getMessageIdForEvent($itemtype, $items_id, $event);
$this->assertMatchesRegularExpression($expected, $messageid);
}

public function testGetTargetsWithExclusions()
{
/** @var \DBmysql $DB */
global $DB;

$notification_target = new \NotificationTarget();
$group = new \Group();
$user = new \User();

// Create new user with a fake email
$this->assertGreaterThan(0, $users_id = $user->add([
'name' => __FUNCTION__,
]));
$useremail = new \UserEmail();
$this->assertGreaterThan(0, $useremail->add([
'users_id' => $users_id,
'email' => __FUNCTION__ . '@localhost',
'is_default' => 1,
]));
// Create a new group for this user
$this->assertGreaterThan(0, $groups_id = $group->add([
'name' => __FUNCTION__,
]));
$group_user = new \Group_User();
$this->assertGreaterThan(0, $group_user->add([
'groups_id' => $groups_id,
'users_id' => $users_id,
]));

$notification = new \Notification();
$this->assertGreaterThan(0, $fake_notification_id = $notification->add([
'itemtype' => 'Ticket',
'event' => 'new',
]));
$notification_target->data = [
'notifications_id' => $fake_notification_id,
];
$rc = new \ReflectionClass($notification_target);
$rc->getProperty('event')->setValue($notification_target, \NotificationEventMailing::class);

$notification_target->addToRecipientsList([
'users_id' => getItemByTypeName('User', TU_USER, true),
'usertype' => \NotificationTarget::GLPI_USER,
]);
$notification_target->addToRecipientsList([
'users_id' => $users_id,
'usertype' => \NotificationTarget::GLPI_USER,
]);
$this->assertCount(2, $notification_target->getTargets());

$this->assertGreaterThan(0, $notification_target->add([
'notifications_id' => $fake_notification_id,
'type' => \Notification::GROUP_TYPE,
'items_id' => $groups_id,
'is_exclusion' => 1,
]));
// Only TU_USER should be in the list
$targets = $notification_target->getTargets();
$this->assertCount(1, $targets);
$target = reset($targets);
$this->assertEquals(getItemByTypeName('User', TU_USER, true), $target['users_id']);
}
}
125 changes: 119 additions & 6 deletions src/NotificationTarget.php
Original file line number Diff line number Diff line change
Expand Up @@ -438,27 +438,46 @@ public function showForNotification(Notification $notification)
$canedit = $notification->can($notifications_id, UPDATE);

$values = [];
$allowed_exclusion_types = [
Notification::PROFILE_TYPE,
Notification::GROUP_TYPE,
];
$all_exclusion_targets = [];
foreach ($this->notification_targets as $key => $val) {
[$type, $id] = explode('_', $key);
$values[$key] = $this->notification_targets_labels[$type][$id];
$label = $this->notification_targets_labels[$type][$id];
if (in_array((int) $type, $allowed_exclusion_types, true)) {
$all_exclusion_targets[$key] = $label;
} else {
$values[$key] = $label;
}
}

$targets = getAllDataFromTable(
self::getTable(),
[
'notifications_id' => $notifications_id
]
);
$actives = [];
$exclusions = [];
if (count($targets)) {
foreach ($targets as $data) {
$actives[$data['type'] . '_' . $data['items_id']] = $data['type'] . '_' . $data['items_id'];
$target_key = $data['type'] . '_' . $data['items_id'];
if ($data['is_exclusion']) {
$exclusions[$target_key] = $target_key;
} else {
$actives[$target_key] = $target_key;
}
}
}
TemplateRenderer::getInstance()->display('pages/setup/notification/recipients.html.twig', [
'item' => $this,
'notification' => $notification,
'all_targets' => $values,
'all_exclusion_targets' => $all_exclusion_targets,
'active_targets' => $actives,
'excluded_targets' => $exclusions,
'params' => [
'canedit' => $canedit
]
Expand Down Expand Up @@ -512,7 +531,26 @@ public static function updateTargets($input)
}
}

// Drop others
if (isset($input['_exclusions']) && count($input['_exclusions'])) {
$input['_exclusions'] = array_unique($input['_exclusions']);
// Remove exclusions already set in _targets
$input['_exclusions'] = array_diff($input['_exclusions'], $input['_targets'] ?? []);
foreach ($input['_exclusions'] as $val) {
// Add if not set
if (!isset($actives[$val])) {
list($type, $items_id) = explode("_", $val);
$tmp = [];
$tmp['items_id'] = $items_id;
$tmp['type'] = $type;
$tmp['notifications_id'] = $input['notifications_id'];
$tmp['is_exclusion'] = 1;
$target->add($tmp);
}
unset($actives[$val]);
}
}

// Drop others
if (count($actives)) {
foreach ($actives as $val) {
list($type, $items_id) = explode("_", $val);
Expand Down Expand Up @@ -1348,9 +1386,82 @@ public function addDataForTemplate($event, $options = [])

final public function getTargets()
{
return $this->target;
return $this->removeExcludedTargets($this->target);
}

private function removeExcludedTargets(array $target_list)
{
/** @var \DBmysql $DB */
global $DB;
$exclusions = iterator_to_array($DB->request([
'SELECT' => ['type', 'items_id'],
'FROM' => self::getTable(),
'WHERE' => [
'is_exclusion' => 1,
'notifications_id' => $this->data['notifications_id']
]
]));
if (empty($exclusions)) {
// No exclusion, no need to filter
return $target_list;
}
$user_ids = [];
foreach ($target_list as $target) {
if (isset($target['users_id'])) {
$user_ids[] = $target['users_id'];
}
}
if (empty($user_ids)) {
// Cannot filter targets without a user id
return $target_list;
}

// Criteria to get any user IDs that are excluded
$criteria = [
'SELECT' => [User::getTableField('id')],
'FROM' => User::getTable(),
'INNER JOIN' => [
Profile_User::getTable() => [
'ON' => [
Profile_User::getTable() => 'users_id',
User::getTable() => 'id'
]
],
Group_User::getTable() => [
'ON' => [
Group_User::getTable() => 'users_id',
User::getTable() => 'id'
]
]
],
'WHERE' => [
'OR' => [
[
Profile_User::getTableField('profiles_id') => array_column($exclusions, 'items_id'),
Profile_User::getTableField('users_id') => $user_ids
],
[
Group_User::getTableField('groups_id') => array_column($exclusions, 'items_id'),
Group_User::getTableField('users_id') => $user_ids
]
],
]
];

$excluded_user_ids = [];
$it = $DB->request($criteria);
foreach ($it as $data) {
$excluded_user_ids[] = $data['id'];
}

foreach ($target_list as $key => $target) {
if (isset($target['users_id']) && in_array($target['users_id'], $excluded_user_ids, true)) {
unset($target_list[$key]);
}
}

return $target_list;
}

public function getEntity()
{
Expand Down Expand Up @@ -1568,7 +1679,8 @@ public static function countForGroup(Group $group)
Notification::SUPERVISOR_GROUP_TYPE,
Notification::GROUP_TYPE
],
'items_id' => $group->getID()
'items_id' => $group->getID(),
'is_exclusion' => 0,
] + getEntitiesRestrictCriteria(Notification::getTable(), '', '', true)
])->current();
return $count['cpt'];
Expand Down Expand Up @@ -1609,7 +1721,8 @@ public static function showForGroup(Group $group)
Notification::SUPERVISOR_GROUP_TYPE,
Notification::GROUP_TYPE
],
'items_id' => $group->getID()
'items_id' => $group->getID(),
'is_exclusion' => 0,
] + getEntitiesRestrictCriteria(Notification::getTable(), '', '', true)
]);

Expand Down
6 changes: 6 additions & 0 deletions templates/pages/setup/notification/recipients.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,13 @@
}) }}

{{ fields.nullField() }}
{{ fields.dropdownArrayField('_exclusions', null, all_exclusion_targets, _n('Exclusion', 'Exclusions', get_plural_number()), {
multiple: true,
readonly: not params['canedit'],
values: excluded_targets
}) }}
{{ fields.nullField() }}
{{ fields.nullField() }}
{{ fields.htmlField('', inputs.submit('update', _x('button', 'Update'), 1), '') }}
</div>
</div>
Expand Down
3 changes: 2 additions & 1 deletion tests/functional/NotificationTargetKnowbaseItem.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ public function testgetDataForNotifKnowbaseItem()
'id' => $ntarget->fields['id'],
'items_id' => $group->fields['id'],
'type' => Notification::GROUP_TYPE,
'notifications_id' => $kbnotif['id']
'notifications_id' => $kbnotif['id'],
'is_exclusion' => 0,
]
);
}
Expand Down

0 comments on commit ff21253

Please sign in to comment.