Unverified Commit b12dccd4 authored by Cédric Anne's avatar Cédric Anne Committed by GitHub
Browse files

Fix handling of ITIL documents (#7060)

* Fix missing documents in ITIL timeline; fixes #7059

* Fix timeline document items deletion

* Fix documents in ITIL notifications

* Return followup and task documents in Ticket/Change/Problem API

* Add missing solution documents

* Fix tranfer of itil sub items documents

* Fix rights checks on ITIL documents

* Apply suggestions from code review

* Fix rights checks and add some tests
parent e82617db
......@@ -105,13 +105,13 @@ if (isset($_POST["add"])) {
sprintf(__('%s adds an actor'), $_SESSION["glpiname"]));
Html::redirect(Change::getFormURLWithID($_POST['changes_id']));
} else if (isset($_REQUEST['delete_document'])) {
$change->getFromDB((int)$_REQUEST['changes_id']);
$doc = new Document();
$doc->getFromDB(intval($_REQUEST['documents_id']));
if ($doc->can($doc->getID(), UPDATE)) {
$document_item = new Document_Item;
$found_document_items = $document_item->find([
'itemtype' => 'Change',
'items_id' => (int)$_REQUEST['changes_id'],
$change->getAssociatedDocumentsCriteria(),
'documents_id' => $doc->getID()
]);
foreach ($found_document_items as $item) {
......
......@@ -108,13 +108,13 @@ if (isset($_POST["add"])) {
sprintf(__('%s adds an actor'), $_SESSION["glpiname"]));
Html::redirect($problem->getFormURLWithID($_POST['problems_id']));
} else if (isset($_REQUEST['delete_document'])) {
$problem->getFromDB((int)$_REQUEST['problems_id']);
$doc = new Document();
$doc->getFromDB(intval($_REQUEST['documents_id']));
if ($doc->can($doc->getID(), UPDATE)) {
$document_item = new Document_Item;
$found_document_items = $document_item->find([
'itemtype' => 'Problem',
'items_id' => (int)$_REQUEST['problems_id'],
$problem->getAssociatedDocumentsCriteria(),
'documents_id' => $doc->getID()
]);
foreach ($found_document_items as $item) {
......
......@@ -185,13 +185,13 @@ if (isset($_POST["add"])) {
sprintf(__('%s adds an actor'), $_SESSION["glpiname"]));
Html::redirect(Ticket::getFormURLWithID($_POST['tickets_id']));
} else if (isset($_REQUEST['delete_document'])) {
$track->getFromDB((int)$_REQUEST['tickets_id']);
$doc = new Document();
$doc->getFromDB(intval($_REQUEST['documents_id']));
if ($doc->can($doc->getID(), UPDATE)) {
$document_item = new Document_Item;
$found_document_items = $document_item->find([
'itemtype' => 'Ticket',
'items_id' => (int)$_REQUEST['tickets_id'],
$track->getAssociatedDocumentsCriteria(),
'documents_id' => $doc->getID()
]);
foreach ($found_document_items as $item) {
......
......@@ -929,6 +929,16 @@ abstract class API extends CommonGLPI {
&& !Document::canView()) {
$fields['_documents'] = self::arrayRightError();
} else {
$doc_criteria = [
'glpi_documents_items.items_id' => $id,
'glpi_documents_items.itemtype' => $itemtype
];
if ($item instanceof CommonITILObject) {
$doc_criteria = [
$item->getAssociatedDocumentsCriteria(),
'timeline_position' => ['>', CommonITILObject::NO_TIMELINE], // skip inlined images
];
}
$doc_iterator = $DB->request([
'SELECT' => [
'glpi_documents_items.id AS assocID',
......@@ -959,10 +969,7 @@ abstract class API extends CommonGLPI {
]
]
],
'WHERE' => [
'glpi_documents_items.items_id' => $id,
'glpi_documents_items.itemtype' => $itemtype
]
'WHERE' => $doc_criteria,
]);
while ($data = $doc_iterator->next()) {
$fields['_documents'][] = $data;
......
......@@ -6741,8 +6741,7 @@ abstract class CommonITILObject extends CommonDBTM {
//add documents to timeline
$document_obj = new Document();
$document_items = $document_item_obj->find([
'itemtype' => $objType,
'items_id' => $this->getID(),
$this->getAssociatedDocumentsCriteria(),
'timeline_position' => ['>', self::NO_TIMELINE]
]);
foreach ($document_items as $document_item) {
......@@ -7880,4 +7879,85 @@ abstract class CommonITILObject extends CommonDBTM {
}
return $excluded;
}
/**
* Returns criteria that can be used to get documents related to current instance.
*
* @return array
*/
public function getAssociatedDocumentsCriteria($bypass_rights = false): array {
$task_class = $this->getType() . 'Task';
$or_crits = [
// documents associated to ITIL item directly
[
Document_Item::getTableField('itemtype') => $this->getType(),
Document_Item::getTableField('items_id') => $this->getID(),
],
];
// documents associated to followups
if ($bypass_rights || ITILFollowup::canView()) {
$fup_crits = [
ITILFollowup::getTableField('itemtype') => $this->getType(),
ITILFollowup::getTableField('items_id') => $this->getID(),
];
if (!$bypass_rights && !Session::haveRight(ITILFollowup::$rightname, ITILFollowup::SEEPRIVATE)) {
$fup_crits[] = [
'OR' => ['is_private' => 0, 'users_id' => Session::getLoginUserID()],
];
}
$or_crits[] = [
Document_Item::getTableField('itemtype') => ITILFollowup::getType(),
Document_Item::getTableField('items_id') => new QuerySubQuery(
[
'SELECT' => 'id',
'FROM' => ITILFollowup::getTable(),
'WHERE' => $fup_crits,
]
),
];
}
// documents associated to solutions
if (ITILSolution::canView()) {
$or_crits[] = [
Document_Item::getTableField('itemtype') => ITILSolution::getType(),
Document_Item::getTableField('items_id') => new QuerySubQuery(
[
'SELECT' => 'id',
'FROM' => ITILSolution::getTable(),
'WHERE' => [
ITILSolution::getTableField('itemtype') => $this->getType(),
ITILSolution::getTableField('items_id') => $this->getID(),
],
]
),
];
}
// documents associated to tasks
if ($bypass_rights || $task_class::canView()) {
$tasks_crit = [
$this->getForeignKeyField() => $this->getID(),
];
if (!$bypass_rights && !Session::haveRight($task_class::$rightname, CommonITILTask::SEEPRIVATE)) {
$tasks_crit[] = [
'OR' => ['is_private' => 0, 'users_id' => Session::getLoginUserID()],
];
}
$or_crits[] = [
'glpi_documents_items.itemtype' => $task_class::getType(),
'glpi_documents_items.items_id' => new QuerySubQuery(
[
'SELECT' => 'id',
'FROM' => $task_class::getTable(),
'WHERE' => $tasks_crit,
]
),
];
}
return ['OR' => $or_crits];
}
}
......@@ -772,55 +772,17 @@ class Document extends CommonDBTM {
return false;
}
$itil->getFromDB($items_id);
$result = $DB->request([
'FROM' => Document_Item::getTable(),
'COUNT' => 'cpt',
'WHERE' => [
'items_id' => $items_id,
'itemtype' => $itemtype,
$itil->getAssociatedDocumentsCriteria(),
'documents_id' => $this->fields['id']
]
])->next();
if ($result['cpt'] > 0) {
return true;
}
// Check ticket and child items (followups, tasks, solutions) contents
$itil_table = $itil->getTable();
$itil_key = $itil->getForeignKeyField();
$task_table = getTableForItemType($itil->getType() . 'Task');
$result = $DB->request([
'FROM' => $itil_table,
'COUNT' => 'cpt',
'LEFT JOIN' => [
'glpi_itilfollowups' => [
'FKEY' => [
$itil_table => 'id',
'glpi_itilfollowups' => 'items_id',
['AND' => ['glpi_itilfollowups.itemtype' => $itemtype]]
]
],
$task_table => [
'FKEY' => [
$itil_table => 'id',
$task_table => $itil_key
]
],
'glpi_itilsolutions' => [
'FKEY' => [
$itil_table => 'id',
'glpi_itilsolutions' => 'items_id',
['AND' => ['glpi_itilsolutions.itemtype' => $itemtype]]
]
],
],
'WHERE' => [
$itil_table . '.id' => $items_id,
]
])->next();
return $result['cpt'] > 0;
}
......
......@@ -144,14 +144,28 @@ class NotificationEventMailing extends NotificationEventAbstract implements Noti
if ($is_html || $CFG_GLPI['attach_ticket_documents_to_mail']) {
// Retieve document list if mail is in HTML format (for inline images)
// or if documents are attached to mail.
$item = getItemForItemtype($current->fields['itemtype']);
$doc_crit = [
'items_id' => $current->fields['items_id'],
'itemtype' => $current->fields['itemtype'],
];
if ($item instanceof CommonITILObject) {
$item->getFromDB($current->fields['items_id']);
$doc_crit = $item->getAssociatedDocumentsCriteria();
if ($is_html) {
// Remove documents having "NO_TIMELINE" position if mail is HTML, as
// these documents corresponds to inlined images.
// If notification is in plain text, they should be kepts as they cannot be rendered in text.
$doc_crit[] = [
'timeline_position' => ['>', CommonITILObject::NO_TIMELINE]
];
}
}
$doc_items_iterator = $DB->request(
[
'SELECT' => ['documents_id'],
'FROM' => Document_Item::getTable(),
'WHERE' => [
'items_id' => $current->fields['items_id'],
'itemtype' => $current->fields['itemtype'],
]
'WHERE' => $doc_crit,
]
);
foreach ($doc_items_iterator as $doc_item) {
......
......@@ -1296,8 +1296,8 @@ abstract class NotificationTargetCommonITILObject extends NotificationTarget {
]
],
'WHERE' => [
'glpi_documents_items.itemtype' => $item->getType(),
'glpi_documents_items.items_id' => $item->fields['id']
$item->getAssociatedDocumentsCriteria(),
'timeline_position' => ['>', CommonITILObject::NO_TIMELINE], // skip inlined images
]
]);
......
......@@ -1105,6 +1105,23 @@ class Transfer extends CommonDBTM {
// Document : keep / delete + clean unused / keep unused
if (Document::canApplyOn($itemtype)) {
$this->transferDocuments($itemtype, $ID, $newID);
if (is_a($itemtype, CommonITILObject::class, true)) {
// Transfer ITIL childs documents too
$itil_item = getItemForItemtype($itemtype);
$itil_item->getFromDB($ID);
$document_item_obj = new Document_Item();
$document_items = $document_item_obj->find(
$itil_item->getAssociatedDocumentsCriteria(true)
);
foreach ($document_items as $document_item) {
$this->transferDocuments(
$document_item['itemtype'],
$document_item['items_id'],
$document_item['items_id']
);
}
}
}
// Transfer compatible printers
......
......@@ -2912,4 +2912,129 @@ class Ticket extends DbTestCase {
// Target ticket should have all suppliers not marked as duplicates above
$this->integer((int)$supplier_count)->isEqualTo(3);
}
/**
* @see self::testGetAssociatedDocumentsCriteria()
*/
protected function getAssociatedDocumentsCriteriaProvider() {
$ticket = new \Ticket();
$ticket_id = $ticket->add([
'name' => "test",
'content' => "test",
]);
$this->integer((int)$ticket_id)->isGreaterThan(0);
return [
[
'rights' => [
\Change::$rightname => 0,
\Problem::$rightname => 0,
\Ticket::$rightname => 0,
\ITILFollowup::$rightname => 0,
\TicketTask::$rightname => 0,
],
'ticket_id' => $ticket_id,
'bypass_rights' => false,
'expected_where' => sprintf(
"(`glpi_documents_items`.`itemtype` = 'Ticket' AND `glpi_documents_items`.`items_id` = '%1\$s')",
$ticket_id
),
],
[
'rights' => [
\Change::$rightname => 0,
\Problem::$rightname => 0,
\Ticket::$rightname => \READ,
\ITILFollowup::$rightname => 0,
\TicketTask::$rightname => 0,
],
'ticket_id' => $ticket_id,
'bypass_rights' => false,
'expected_where' => sprintf(
"(`glpi_documents_items`.`itemtype` = 'Ticket' AND `glpi_documents_items`.`items_id` = '%1\$s')"
. " OR (`glpi_documents_items`.`itemtype` = 'ITILFollowup' AND `glpi_documents_items`.`items_id` IN (SELECT `id` FROM `glpi_itilfollowups` WHERE `glpi_itilfollowups`.`itemtype` = 'Ticket' AND `glpi_itilfollowups`.`items_id` = '%1\$s' AND ((`is_private` = '0' OR `users_id` = '%2\$s'))))"
. " OR (`glpi_documents_items`.`itemtype` = 'ITILSolution' AND `glpi_documents_items`.`items_id` IN (SELECT `id` FROM `glpi_itilsolutions` WHERE `glpi_itilsolutions`.`itemtype` = 'Ticket' AND `glpi_itilsolutions`.`items_id` = '%1\$s'))",
$ticket_id,
getItemByTypeName('User', TU_USER, true)
),
],
[
'rights' => [
\Change::$rightname => 0,
\Problem::$rightname => 0,
\Ticket::$rightname => \READ,
\ITILFollowup::$rightname => \ITILFollowup::SEEPUBLIC,
\TicketTask::$rightname => \TicketTask::SEEPUBLIC,
],
'ticket_id' => $ticket_id,
'bypass_rights' => false,
'expected_where' => sprintf(
"(`glpi_documents_items`.`itemtype` = 'Ticket' AND `glpi_documents_items`.`items_id` = '%1\$s')"
. " OR (`glpi_documents_items`.`itemtype` = 'ITILFollowup' AND `glpi_documents_items`.`items_id` IN (SELECT `id` FROM `glpi_itilfollowups` WHERE `glpi_itilfollowups`.`itemtype` = 'Ticket' AND `glpi_itilfollowups`.`items_id` = '%1\$s' AND ((`is_private` = '0' OR `users_id` = '%2\$s'))))"
. " OR (`glpi_documents_items`.`itemtype` = 'ITILSolution' AND `glpi_documents_items`.`items_id` IN (SELECT `id` FROM `glpi_itilsolutions` WHERE `glpi_itilsolutions`.`itemtype` = 'Ticket' AND `glpi_itilsolutions`.`items_id` = '%1\$s'))"
. " OR (`glpi_documents_items`.`itemtype` = 'TicketTask' AND `glpi_documents_items`.`items_id` IN (SELECT `id` FROM `glpi_tickettasks` WHERE `tickets_id` = '%1\$s' AND ((`is_private` = '0' OR `users_id` = '%2\$s'))))",
$ticket_id,
getItemByTypeName('User', TU_USER, true)
),
],
[
'rights' => [
\Change::$rightname => 0,
\Problem::$rightname => 0,
\Ticket::$rightname => \READ,
\ITILFollowup::$rightname => \ITILFollowup::SEEPRIVATE,
\TicketTask::$rightname => 0,
],
'ticket_id' => $ticket_id,
'bypass_rights' => false,
'expected_where' => sprintf(
"(`glpi_documents_items`.`itemtype` = 'Ticket' AND `glpi_documents_items`.`items_id` = '%1\$s')"
. " OR (`glpi_documents_items`.`itemtype` = 'ITILFollowup' AND `glpi_documents_items`.`items_id` IN (SELECT `id` FROM `glpi_itilfollowups` WHERE `glpi_itilfollowups`.`itemtype` = 'Ticket' AND `glpi_itilfollowups`.`items_id` = '%1\$s'))"
. " OR (`glpi_documents_items`.`itemtype` = 'ITILSolution' AND `glpi_documents_items`.`items_id` IN (SELECT `id` FROM `glpi_itilsolutions` WHERE `glpi_itilsolutions`.`itemtype` = 'Ticket' AND `glpi_itilsolutions`.`items_id` = '%1\$s'))",
$ticket_id,
getItemByTypeName('User', TU_USER, true)
),
],
[
'rights' => [
\Change::$rightname => 0,
\Problem::$rightname => 0,
\Ticket::$rightname => \READ,
\ITILFollowup::$rightname => \ITILFollowup::SEEPUBLIC,
\TicketTask::$rightname => \TicketTask::SEEPRIVATE,
],
'ticket_id' => $ticket_id,
'bypass_rights' => false,
'expected_where' => sprintf(
"(`glpi_documents_items`.`itemtype` = 'Ticket' AND `glpi_documents_items`.`items_id` = '%1\$s')"
. " OR (`glpi_documents_items`.`itemtype` = 'ITILFollowup' AND `glpi_documents_items`.`items_id` IN (SELECT `id` FROM `glpi_itilfollowups` WHERE `glpi_itilfollowups`.`itemtype` = 'Ticket' AND `glpi_itilfollowups`.`items_id` = '%1\$s' AND ((`is_private` = '0' OR `users_id` = '%2\$s'))))"
. " OR (`glpi_documents_items`.`itemtype` = 'ITILSolution' AND `glpi_documents_items`.`items_id` IN (SELECT `id` FROM `glpi_itilsolutions` WHERE `glpi_itilsolutions`.`itemtype` = 'Ticket' AND `glpi_itilsolutions`.`items_id` = '%1\$s'))"
. " OR (`glpi_documents_items`.`itemtype` = 'TicketTask' AND `glpi_documents_items`.`items_id` IN (SELECT `id` FROM `glpi_tickettasks` WHERE `tickets_id` = '%1\$s'))",
$ticket_id,
getItemByTypeName('User', TU_USER, true)
),
],
];
}
/**
* @dataProvider getAssociatedDocumentsCriteriaProvider
*/
public function testGetAssociatedDocumentsCriteria($rights, $ticket_id, $bypass_rights, $expected_where) {
$this->login();
$ticket = new \Ticket();
$this->boolean($ticket->getFromDB($ticket_id))->isTrue();
$session_backup = $_SESSION['glpiactiveprofile'];
foreach ($rights as $rightname => $rightvalue) {
$_SESSION['glpiactiveprofile'][$rightname] = $rightvalue;
}
$crit = $ticket->getAssociatedDocumentsCriteria($bypass_rights);
$_SESSION['glpiactiveprofile'] = $session_backup;
$it = new \DBmysqlIterator(null);
$it->execute('glpi_tickets', $crit);
$this->string($it->getSql())->isIdenticalTo('SELECT * FROM `glpi_tickets` WHERE (' . $expected_where . ')');
}
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment