Skip to content

Fix can update item calls #23923

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.

Sign up for GitHub

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

Open
SebSept wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Fix can update item calls #23923

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2ebc85c
rector rule to replace canUpdateItem()
SebSept Apr 15, 2026
108e09d
rector result - only correct fixes out of the box
SebSept Apr 15, 2026
e65cd3d
rector result + manual fix
SebSept Apr 15, 2026
631bca7
Revert "rector rule to replace canUpdateItem()"
SebSept Apr 16, 2026
15929cf
phpstan fixes
SebSept Apr 23, 2026
8bb2255
revert unexpected changes
SebSept Apr 23, 2026
7f6ac9b
cs fix
SebSept Apr 23, 2026
a0eda74
fix test mocks
SebSept Apr 24, 2026
35db2ec
remove useless ->can() + some tiny optimizations
SebSept May 4, 2026
27cca53
fix expected failure count in MassiveActionTest
SebSept May 5, 2026
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
30 changes: 0 additions & 30 deletions .phpstan-baseline.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3247,12 +3247,6 @@
'count' => 1,
'path' => __DIR__ . '/src/CommonITILCost.php',
];
$ignoreErrors[] = [
'message' => '#^Cannot access constant class on CommonITILTask\\|false\\.$#',
'identifier' => 'classConstant.nonObject',
'count' => 1,
'path' => __DIR__ . '/src/CommonITILObject.php',
];
$ignoreErrors[] = [
'message' => '#^Cannot access constant class on CommonITILValidation\\|null\\.$#',
'identifier' => 'classConstant.nonObject',
Expand All @@ -3265,12 +3259,6 @@
'count' => 1,
'path' => __DIR__ . '/src/CommonITILObject.php',
];
$ignoreErrors[] = [
'message' => '#^Cannot access property \\$fields on CommonITILTask\\|false\\.$#',
'identifier' => 'property.nonObject',
'count' => 1,
'path' => __DIR__ . '/src/CommonITILObject.php',
];
$ignoreErrors[] = [
'message' => '#^Cannot access property \\$fields on ITILCategory\\|false\\.$#',
'identifier' => 'property.nonObject',
Expand All @@ -3283,18 +3271,6 @@
'count' => 1,
'path' => __DIR__ . '/src/CommonITILObject.php',
];
$ignoreErrors[] = [
'message' => '#^Cannot call method canUpdateItem\\(\\) on CommonITILTask\\|false\\.$#',
'identifier' => 'method.nonObject',
'count' => 1,
'path' => __DIR__ . '/src/CommonITILObject.php',
];
$ignoreErrors[] = [
'message' => '#^Cannot call method canViewItem\\(\\) on CommonITILTask\\|false\\.$#',
'identifier' => 'method.nonObject',
'count' => 1,
'path' => __DIR__ . '/src/CommonITILObject.php',
];
$ignoreErrors[] = [
'message' => '#^Cannot call method getCriterias\\(\\) on Rule\\|null\\.$#',
'identifier' => 'method.nonObject',
Expand Down Expand Up @@ -3331,12 +3307,6 @@
'count' => 1,
'path' => __DIR__ . '/src/CommonITILObject.php',
];
$ignoreErrors[] = [
'message' => '#^Cannot call method post_getFromDB\\(\\) on CommonITILTask\\|false\\.$#',
'identifier' => 'method.nonObject',
'count' => 1,
'path' => __DIR__ . '/src/CommonITILObject.php',
];
$ignoreErrors[] = [
'message' => '#^Cannot call method update\\(\\) on ITIL_ValidationStep\\|null\\.$#',
'identifier' => 'method.nonObject',
Expand Down
2 changes: 1 addition & 1 deletion ajax/commonitilsatisfaction.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@

if (isset($_POST['inquest_config' . $config_suffix], $_POST['entities_id'])) {
if ($ent->getFromDB($_POST['entities_id'])) {
if (!$ent->canViewItem()) {
if (!$ent->can($ent->getID(), READ)) {
throw new AccessDeniedHttpException();
}
$inquest_delay = $ent->fields['inquest_delay' . $config_suffix];
Expand Down
4 changes: 2 additions & 2 deletions ajax/criteria_filter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
}

// Check rights, must be able to update parent item
if (!$item->canUpdateItem()) {
if (!$item->can($item->getID(), UPDATE)) {
throw new AccessDeniedHttpException('You are not allowed to update this item');
}

Expand Down Expand Up @@ -106,7 +106,7 @@
}

// Check rights, must be able to update parent item
if (!$item->canUpdateItem()) {
if (!$item->can($item->getID(), UPDATE)) {
throw new AccessDeniedHttpException('You are not allowed to update this item');
}

Expand Down
2 changes: 1 addition & 1 deletion ajax/get_item_content.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
// Validate item
if (
!$item->getFromDB($items_id)
|| !$item->canViewItem()
|| !$item->can($item->getID(), READ)
|| !$item->isField('content')
) {
throw new NotFoundHttpException("Item not found");
Expand Down
2 changes: 1 addition & 1 deletion ajax/timeline.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
}

$task = $parent::getTaskClassInstance();
if (!$task->getFromDB((int) $_POST['tasks_id']) || !$task->canUpdateItem()) {
if (!$task->getFromDB((int) $_POST['tasks_id']) || !$task->can($task->getID(), UPDATE)) {
throw new AccessDeniedHttpException();
}
if (!in_array($task->fields['state'], [0, Planning::INFO])) {
Expand Down
2 changes: 1 addition & 1 deletion src/Budget.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public function showForm($ID, array $options = [])
'item' => $this,
'no_header' => true,
'params' => [
'canedit' => $this->canUpdateItem(),
'canedit' => $this->can($this->getID(), UPDATE),
],
]);
return true;
Expand Down
4 changes: 2 additions & 2 deletions src/CommonITILCost.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ public function showForm($ID, array $options = [])
'items_id_field' => static::$items_id,
'parent_id' => $item->getID(),
'params' => [
'canedit' => $this->canUpdateItem(),
'canedit' => $this->can($this->getID(), UPDATE),
],
]);

Expand All @@ -462,7 +462,7 @@ public static function showForObject(CommonITILObject|Project $item): false|floa

if (
!$item->getFromDB($ID)
|| !$item->canViewItem()
|| !$item->can($item->getID(), READ)
|| !static::canView()
) {
return false;
Expand Down
30 changes: 17 additions & 13 deletions src/CommonITILObject.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ public function showForm($ID, array $options = [])

$this->restoreInputAndDefaults($ID, $options);

$canupdate = !$ID || (Session::getCurrentInterface() == "central" && $this->canUpdateItem());
$canupdate = !$ID || (Session::getCurrentInterface() == "central" && $this->can($this->getID(), UPDATE));

if ($ID && in_array($this->fields['status'], static::getClosedStatusArray())) {
$canupdate = false;
Expand Down Expand Up @@ -1855,7 +1855,7 @@ class_exists($validation_class)
}

// Can only update initial fields if no followup or task already added
if ($this->canUpdateItem()) {
if ($this->can($this->getID(), UPDATE)) {
$allowed_fields[] = 'content';
$allowed_fields[] = 'urgency';
$allowed_fields[] = 'priority'; // automatic recalculate if user changes urgence
Expand Down Expand Up @@ -6629,7 +6629,7 @@ public static function showShort($id, $options = [])

// Second column TITLE
$second_column = "<span class='b'>" . htmlescape($item->getName()) . "</span>&nbsp;";
if ($item->canViewItem()) {
if ($item->can($item->getID(), READ)) {
$second_column = sprintf(
__s('%1$s (%2$s)'),
"<a id='" . htmlescape($item::class . $item->getID() . $rand) . "' href=\"" . htmlescape($item->getLinkURL()) . "\">$second_column</a>",
Expand Down Expand Up @@ -7070,7 +7070,7 @@ public static function getDatatableEntries(array $data, $params = []): array
}

$name = '<span class="fw-bold">' . htmlescape($item->getName()) . '</span>';
if ($item->canViewItem()) {
if ($item->can($item->getID(), READ)) {
$name = sprintf(
__s('%1$s (%2$s)'),
'<a id="' . htmlescape($name_link_id) . '" href="' . htmlescape($item->getLinkURL()) . '">' . $name . '</a><br>',
Expand Down Expand Up @@ -7569,7 +7569,7 @@ public function getTimelineItems(array $options = [])
return [];
}

if ($params['check_view_rights'] && !$this->canViewItem()) {
if ($params['check_view_rights'] && !$this->can($this->getID(), READ)) {
return [];
}

Expand All @@ -7578,7 +7578,7 @@ public function getTimelineItems(array $options = [])
$foreignKey = static::getForeignKeyField();
$timeline = [];

$canupdate_parent = $this->canUpdateItem() && !in_array($this->fields['status'], static::getClosedStatusArray());
$canupdate_parent = $this->can($this->getID(), UPDATE) && !in_array($this->fields['status'], static::getClosedStatusArray());

//checks rights
$restrict_fup = $restrict_task = [];
Expand Down Expand Up @@ -7656,8 +7656,8 @@ public function getTimelineItems(array $options = [])
$followup->fields = $followup_row;
$followup->post_getFromDB();

if (!$params['check_view_rights'] || $followup->canViewItem()) {
$followup_row['can_edit'] = $followup->canUpdateItem();
if (!$params['check_view_rights'] || $followup->can($followup->getID(), READ)) {
$followup_row['can_edit'] = $followup->can($followup->getID(), UPDATE);
$followup_row['can_promote']
= Session::getCurrentInterface() === 'central'
&& $this instanceof Ticket
Expand Down Expand Up @@ -7692,11 +7692,14 @@ public function getTimelineItems(array $options = [])
foreach ($tasks as $tasks_id => $task_row) {
// Safer to use a clean object to load our data
$tltask = getItemForItemtype($taskClass);
if (($tltask === false)) {
continue;
}
$tltask->fields = $task_row;
$tltask->post_getFromDB();

if (!$params['check_view_rights'] || $tltask->canViewItem()) {
$task_row['can_edit'] = $tltask->canUpdateItem();
if (!$params['check_view_rights'] || $tltask->can($tltask->getID(), READ)) {
$task_row['can_edit'] = $tltask->can($tltask->getID(), UPDATE);
$task_row['can_promote']
= Session::getCurrentInterface() === 'central'
&& $this instanceof Ticket
Expand Down Expand Up @@ -7879,7 +7882,7 @@ class_exists($validation_class ?? '') && $params['with_validations']
$item['is_private'] = $document_item['is_private'];

$item['timeline_position'] = $document_item['timeline_position'];
$item['_can_edit'] = Document::canUpdate() && $document_obj->canUpdateItem();
$item['_can_edit'] = $document_obj->can($document_obj->getID(), UPDATE);
$item['_can_delete'] = Document::canDelete() && $document_obj->canDeleteItem() && $canupdate_parent;

$timeline_key = $document_item['itemtype'] . "_" . $document_item['items_id'];
Expand Down Expand Up @@ -8034,7 +8037,7 @@ public static function showEditDescriptionForm(CommonITILObject $item)
$can_requester = $item->canRequesterUpdateItem();
TemplateRenderer::getInstance()->display('components/itilobject/timeline/simple_form.html.twig', [
'item' => $item,
'canupdate' => (Session::getCurrentInterface() == "central" && $item->canUpdateItem()),
'canupdate' => (Session::getCurrentInterface() == "central" && $item->can($item->getID(), UPDATE)),
'can_requester' => $can_requester,
]);
}
Expand Down Expand Up @@ -11137,6 +11140,7 @@ protected function transformActorsInput(array $input): array
$this->loadActors();
}

$can_update_item = $this->can($this->getID(), UPDATE);
if (
array_key_exists('_actors', $input)
&& is_array($input['_actors'])
Expand All @@ -11147,7 +11151,7 @@ protected function transformActorsInput(array $input): array
if ($actor_type_value === CommonITILActor::ASSIGN && !$this->canAssign()) {
continue;
}
if ($actor_type_value !== CommonITILActor::ASSIGN && !$this->isNewItem() && !$this->canUpdateItem()) {
if ($actor_type_value !== CommonITILActor::ASSIGN && !$this->isNewItem() && !$can_update_item) {
continue;
}

Expand Down
6 changes: 3 additions & 3 deletions src/CommonITILTask.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1476,7 +1476,7 @@ public static function genericPopulatePlanning($itemtype, $options = [])
if (count($iterator)) {
foreach ($iterator as $data) {
$item->getFromResultSet($data);
if ($item->canViewItem()) {
if ($item->can($item->getID(), READ)) {
Comment thread
SebSept marked this conversation as resolved.
if ($parentitem->getFromDBwithData($item->fields[$parentitem->getForeignKeyField()])) {
//not planned
if (isset($data['notp_date'])) {
Expand Down Expand Up @@ -1537,7 +1537,7 @@ public static function genericPopulatePlanning($itemtype, $options = [])
$interv[$key]["status"] = $parentitem->fields["status"];
$interv[$key]["priority"] = $parentitem->fields["priority"];

$interv[$key]["editable"] = $item->canUpdateItem();
$interv[$key]["editable"] = $item->can($item->getID(), UPDATE);

/// Specific for tickets
$interv[$key]["device"] = [];
Expand Down Expand Up @@ -2065,7 +2065,7 @@ public function getAsVCalendar()

global $CFG_GLPI;

if (!$this->canViewItem()) {
if (!$this->can($this->getID(), READ)) {
return null;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Consumable.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ public static function displayConsumableList(ConsumableItem $parent): void
'item' => new self(),
'parent' => $parent,
'itemtype' => self::class,
'can_edit' => $parent->canUpdate() && $parent->canUpdateItem(),
'can_edit' => $parent->can($parent->getID(), UPDATE),
'criteria_unused' => $criteria_unused,
'criteria_used' => $criteria_used,
'count_unused' => $count_unused,
Expand Down
2 changes: 1 addition & 1 deletion src/ContractCost.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ public function showForm($ID, array $options = [])
'items_id_field' => static::$items_id,
'parent_id' => $this->fields['contracts_id'],
'params' => [
'canedit' => $this->canUpdateItem(),
'canedit' => $this->can($this->getID(), UPDATE),
],
]);

Expand Down
2 changes: 1 addition & 1 deletion src/DatabaseInstance.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ public function showForm($ID, array $options = [])
TemplateRenderer::getInstance()->display('pages/management/databaseinstance.html.twig', [
'item' => $this,
'params' => [
'canedit' => $this->canUpdateItem(),
'canedit' => $this->can($this->getID(), UPDATE),
],
]);

Expand Down
4 changes: 2 additions & 2 deletions src/Document.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ public function showForm($ID, array $options = [])
'uploader' => $this->fields['users_id'] > 0 ? getUserLink($this->fields["users_id"]) : '',
'uploaded_files' => self::getUploadedFiles(),
'params' => [
'canedit' => $this->canUpdateItem(),
'canedit' => $this->can($this->getID(), UPDATE),
],
]);

Expand Down Expand Up @@ -808,7 +808,7 @@ private function canViewFileFromItem($itemtype, $items_id): bool

/** @var CommonDBTM $item */
$item->getFromDB($items_id);
if (!$item->canViewItem()) {
if (!$item->can($item->getID(), READ)) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Dropdown.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ public static function show($itemtype, $options = [])
if (
$params['value']
&& $item->getFromDB($params['value'])
&& $item->canViewItem()
&& $item->can($item->getID(), READ)
) {
$options_tooltip['link'] = $item->getLinkURL();
} else {
Expand Down
3 changes: 1 addition & 2 deletions src/Entity.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3295,8 +3295,7 @@ public function showHelpdeskHomeConfig(): bool
// Is not root entity
$this->getId() !== 0
// Editable
&& static::canUpdate()
&& $this->canUpdateItem()
&& $this->can($this->getID(), UPDATE)
// Has no tiles
&& count($tiles_manager->getTilesForItem($this)) === 0
) {
Expand Down
6 changes: 3 additions & 3 deletions src/Glpi/Controller/Config/Helpdesk/AbstractTileController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ protected function getAndValidateLinkedItemFromDatabase(
// Invalid database data
throw new RuntimeException();
}
if (!$linked_item::canUpdate() || !$linked_item->canUpdateItem()) {
if (!$linked_item->can($linked_item->getID(), UPDATE)) {
Comment thread
SebSept marked this conversation as resolved.
throw new AccessDeniedHttpException();
}

Expand All @@ -81,7 +81,7 @@ protected function getAndValidateLinkedEntityFromRequest(
if (!$linked_entity->getFromDB($linked_entity_id)) {
throw new BadRequestHttpException();
}
if (!$linked_entity::canUpdate() || !$linked_entity->canUpdateItem()) {
if (!$linked_entity->can($linked_entity->getID(), UPDATE)) {
Comment thread
SebSept marked this conversation as resolved.
throw new AccessDeniedHttpException();
}

Expand All @@ -99,7 +99,7 @@ protected function getAndValidateLinkedItemFromRequest(
) {
throw new BadRequestHttpException();
}
if (!$linked_item::canUpdate() || !$linked_item->canUpdateItem()) {
if (!$linked_item->can($linked_item->getID(), UPDATE)) {
Comment thread
SebSept marked this conversation as resolved.
throw new AccessDeniedHttpException();
}

Expand Down
2 changes: 1 addition & 1 deletion src/Glpi/Controller/Config/Helpdesk/ShowEditTileFormController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public function __invoke(Request $request): Response
$request->query->getString('tile_itemtype'),
$request->query->getInt('tile_id'),
);
if (!$tile::canUpdate() || !$tile->canUpdateItem()) {
if (!$tile->can($tile->getID(), UPDATE)) {
throw new AccessDeniedHttpException();
}

Expand Down
2 changes: 1 addition & 1 deletion src/Glpi/Controller/Config/Helpdesk/UpdateTileController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public function __invoke(Request $request): Response
$request->request->getString('_itemtype'),
$request->request->getInt('id'),
);
if (!$tile::canUpdate() || !$tile->canUpdateItem()) {
if (!$tile->can($tile->getID(), UPDATE)) {
throw new AccessDeniedHttpException();
}

Expand Down
2 changes: 1 addition & 1 deletion src/Glpi/Controller/Form/Destination/GetDestinationFormController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public function __invoke(Request $request, int $form_id, int $destination_id): R
$twig = TemplateRenderer::getInstance()->render('pages/admin/form/form_destination_form.html.twig', [
'destination' => $destination,
'form' => $destination->getForm(),
'can_update' => FormDestination::canUpdate() && $destination->canUpdateItem(),
'can_update' => $destination->can($destination->getID(), UPDATE),
'concrete_destination' => $destination->getConcreteDestinationItem(),
]);
return new Response($twig);
Expand Down
Loading
Loading