Skip to content
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
37 changes: 31 additions & 6 deletions src/CommonDBChild.php
Original file line number Diff line number Diff line change
Expand Up @@ -435,12 +435,36 @@ public function prepareInputForAdd($input)
return false;
}

// Check item exists
if (
static::$mustBeAttached
&& !$this->getItemFromArray(static::$itemtype, static::$items_id, $input)
) {
return false;
if (!$this->getItemFromArray(static::$itemtype, static::$items_id, $input)) {
// The parent item is invalid.

if (static::$mustBeAttached) {
// A valid parent item is mandatory, so creation is blocked with an error message.
$linked_itemtype = preg_match('/^itemtype/', static::$itemtype)
? ($input[static::$itemtype] ?? null)
: static::$itemtype
;
$linked_items_id = $input[static::$items_id] ?? null;

Session::addMessageAfterRedirect(
htmlescape(sprintf(
__('Parent item %s #%s is invalid.'),
is_a($linked_itemtype, CommonDBTM::class, true) ? $linked_itemtype::getTypeName(1) : ($linked_itemtype ?? 'null'),
$linked_items_id ?? 'null'
)),
false,
ERROR
);
return false;
} else {
// A valid parent is not mandatory, so invalid input is cleaned.
if (array_key_exists(static::$itemtype, $input) && preg_match('/^itemtype/', static::$itemtype)) {
$input[static::$itemtype] = ''; // `itemtype` fields are usually not nullable, a default value must be set
}
if (array_key_exists(static::$items_id, $input)) {
$input[static::$items_id] = 0; // foreign key fields may be not nullable, a default value must be set
}
}
}

return $this->addNeededInfoToInput($input);
Expand All @@ -460,6 +484,7 @@ public function prepareInputForUpdate($input)
static::$items_id,
])
) {
// A message is already added by `self::checkAttachedItemChangesAllowed()`
return false;
}

Expand Down
27 changes: 27 additions & 0 deletions src/Glpi/Dashboard/Dashboard.php
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,33 @@ public function getFromDB($ID)
);
}

if (\is_numeric($ID)) {
// Search also on the `id` field.
// This is mandatory to handle the `$this->getFromDB($this->getID());` reload case.
$iterator = $DB->request([
'FROM' => self::getTable(),
'WHERE' => [
'id' => $ID,
],
'LIMIT' => 1,
]);
if (count($iterator) == 1) {
$this->fields = $iterator->current();
$this->key = $this->fields['key'];
$this->post_getFromDB();
return true;
} elseif (count($iterator) > 1) {
throw new TooManyResultsException(
sprintf(
'`%1$s::getFromDB()` expects to get one result, %2$s found in query "%3$s".',
static::class,
count($iterator),
$iterator->getSql()
)
);
}
}

return false;
}

Expand Down
4 changes: 0 additions & 4 deletions src/Glpi/Dashboard/Item.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,6 @@ class Item extends CommonDBChild
public static $itemtype = Dashboard::class;
public static $items_id = 'dashboards_dashboards_id';

// prevent bad getFromDB when bootstraping tests suite
// FIXME Should be true
public static $mustBeAttached = false;

/**
* Return items for the provided dashboard
*
Expand Down
157 changes: 157 additions & 0 deletions tests/functional/CommonDBChildTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
<?php

/**
* ---------------------------------------------------------------------
*
* GLPI - Gestionnaire Libre de Parc Informatique
*
* http://glpi-project.org
*
* @copyright 2015-2025 Teclib' and contributors.
* @licence https://www.gnu.org/licenses/gpl-3.0.html
*
* ---------------------------------------------------------------------
*
* LICENSE
*
* This file is part of GLPI.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*
* ---------------------------------------------------------------------
*/

namespace tests\units;

use CommonDBChild;
use Computer;
use DbTestCase;

class CommonDBChildTest extends DbTestCase
{
public function testPrepareInputForAddWithMandatoryFkeyRelation(): void
{
$instance = new class extends CommonDBChild {
public static $itemtype = Computer::class;
public static $items_id = 'computers_id';
public static $mustBeAttached = true;

public static $disableAutoEntityForwarding = true; // prevent DB accesses
};

$this->assertFalse($instance->prepareInputForAdd(['foo' => 'bar']));
$this->hasSessionMessages(ERROR, ['Parent item Computer #null is invalid.']);

$this->assertFalse($instance->prepareInputForAdd(['computers_id' => 9999999999, 'foo' => 'bar']));
$this->hasSessionMessages(ERROR, ['Parent item Computer #9999999999 is invalid.']);

$valid_id = \getItemByTypeName(Computer::class, '_test_pc01', true);
$this->assertEquals(
['computers_id' => $valid_id, 'foo' => 'bar'],
$instance->prepareInputForAdd(['computers_id' => $valid_id, 'foo' => 'bar'])
);
}

public function testPrepareInputForAddWithOptionalFkeyRelation(): void
{
$instance = new class extends CommonDBChild {
public static $itemtype = Computer::class;
public static $items_id = 'computers_id';
public static $mustBeAttached = false;

public static $disableAutoEntityForwarding = true; // prevent DB accesses
};

$this->assertEquals(
['foo' => 'bar'],
$instance->prepareInputForAdd(['foo' => 'bar'])
);

$this->assertEquals(
['computers_id' => 0, 'foo' => 'bar'],
$instance->prepareInputForAdd(['computers_id' => 9999999999, 'foo' => 'bar'])
);

$valid_id = \getItemByTypeName(Computer::class, '_test_pc01', true);
$this->assertEquals(
['computers_id' => $valid_id, 'foo' => 'bar'],
$instance->prepareInputForAdd(['computers_id' => $valid_id, 'foo' => 'bar'])
);
}
public function testPrepareInputForAddWithMandatoryPolymorphicRelation(): void
{
$instance = new class extends CommonDBChild {
public static $itemtype = 'itemtype';
public static $items_id = 'items_id';
public static $mustBeAttached = true;

public static $disableAutoEntityForwarding = true; // prevent DB accesses
};

$this->assertFalse($instance->prepareInputForAdd(['foo' => 'bar']));
$this->hasSessionMessages(ERROR, ['Parent item null #null is invalid.']);

$this->assertFalse($instance->prepareInputForAdd(['itemtype' => 'Computer', 'foo' => 'bar']));
$this->hasSessionMessages(ERROR, ['Parent item Computer #null is invalid.']);

$this->assertFalse($instance->prepareInputForAdd(['itemtype' => 'NotAClass', 'foo' => 'bar']));
$this->hasSessionMessages(ERROR, ['Parent item NotAClass #null is invalid.']);

$this->assertFalse($instance->prepareInputForAdd(['itemtype' => 'Computer', 'items_id' => 9999999999, 'foo' => 'bar']));
$this->hasSessionMessages(ERROR, ['Parent item Computer #9999999999 is invalid.']);

$valid_id = \getItemByTypeName(Computer::class, '_test_pc01', true);
$this->assertEquals(
['itemtype' => 'Computer', 'items_id' => $valid_id, 'foo' => 'bar'],
$instance->prepareInputForAdd(['itemtype' => 'Computer', 'items_id' => $valid_id, 'foo' => 'bar'])
);
}

public function testPrepareInputForAddWithOptionalPolymorphicRelation(): void
{
$instance = new class extends CommonDBChild {
public static $itemtype = 'itemtype';
public static $items_id = 'items_id';
public static $mustBeAttached = false;

public static $disableAutoEntityForwarding = true; // prevent DB accesses
};

$this->assertEquals(
['foo' => 'bar'],
$instance->prepareInputForAdd(['foo' => 'bar'])
);

$this->assertEquals(
['items_id' => 0, 'foo' => 'bar'],
$instance->prepareInputForAdd(['items_id' => 9999999999, 'foo' => 'bar'])
);

$this->assertEquals(
['itemtype' => '', 'foo' => 'bar'],
$instance->prepareInputForAdd(['itemtype' => 'NotAClass', 'foo' => 'bar'])
);

$this->assertEquals(
['itemtype' => '', 'items_id' => 0, 'foo' => 'bar'],
$instance->prepareInputForAdd(['itemtype' => 'Computer', 'items_id' => 9999999999, 'foo' => 'bar'])
);

$valid_id = \getItemByTypeName(Computer::class, '_test_pc01', true);
$this->assertEquals(
['itemtype' => 'Computer', 'items_id' => $valid_id, 'foo' => 'bar'],
$instance->prepareInputForAdd(['itemtype' => 'Computer', 'items_id' => $valid_id, 'foo' => 'bar'])
);
}
}
8 changes: 6 additions & 2 deletions tests/functional/Glpi/Dashboard/DashboardTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,15 @@ public function testLoad()

public function testGetFromDB()
{
// we need to test we get the dashboard by it's key and not it's id
$this->assertFalse($this->dashboard->getFromDB(1));
// get the dashboard by its key
$this->assertTrue($this->dashboard->getFromDB('test_dashboard'));
$this->assertEquals('test_dashboard', $this->getPrivateProperty('key'));
$this->assertNotEmpty($this->getPrivateProperty('fields'));

// get the dashboard by its id
$dashboard = $this->createItem(Dashboard::class, ['key' => 'test_by_id', 'name' => __FUNCTION__]);
$this->assertTrue($this->dashboard->getFromDB($dashboard->getID()));
$this->assertEquals('test_by_id', $this->getPrivateProperty('key'));
}


Expand Down
1 change: 1 addition & 0 deletions tests/functional/RuleCriteriaTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ public function testPrepareInputForAdd()
// Expects prepareInputForAdd to return false if `rules_id` is not a valid ID
$input = ['rules_id' => $rules_id + 1000, 'criteria' => 'name'];
$this->assertFalse($criteria->prepareInputForAdd($input));
$this->hasSessionMessages(ERROR, ['Parent item Rule #' . ($rules_id + 1000) . ' is invalid.']);
}

public function testGetSearchOptionsNew()
Expand Down