Fix networkport connection log creation #21493
Conversation
Also fix Twig display closes glpi-project#21488
| public static $itemtype = 'NetworkPort'; | ||
| public static $items_id = 'networkports_id'; | ||
| public $dohistory = false; | ||
| public static $mustBeAttached = false; |
There was a problem hiding this comment.
This means that a log can be attached to no NetworkPort. What is the use case ?
There was a problem hiding this comment.
There is no use case. I do not understand why existing this change is needed. CommonDBChild prevents adding log.
There was a problem hiding this comment.
It probably means that a add or update operation does correctly pass the items_id value. If this value does not correspond to a valid NetworkPort ID, then it will prevent it to be added.
Change proposed in #21524 may help. For instance, I had not yet tried to check if it is a bug or an expected message, but adding this message makes the following test fail:
1) tests\units\Glpi\Inventory\Asset\NetworkPortTest::testNetworkEquipmentsConnectionsConverted
Some messages has not been handled in tests\units\Glpi\Inventory\Asset\NetworkPortTest::GLPITestCase::tearDown:
Array
(
[1] => Array
(
[0] => Parent item is invalid.
[1] => Parent item is invalid.
)
)
There was a problem hiding this comment.
Isn't the $items_id property wrong? It is set as "networkports_id" but it should be linked by "networkports_id_source" and "networkports_id_destination".
NetworkPort_NetworkPort::storeConnectionLog tries adding an entry with this input:
$input = [
'networkports_id_source' => $netports_id,
'networkports_id_destination' => $opposite_port,
'connected' => ($action === 'add'),
'date' => $_SESSION['glpi_currenttime'],
];May be why there is a FIXME to change it to extend CommonDBRelation instead of CommonDBChild.
There was a problem hiding this comment.
It seem the incorrect field name is the reason of the initial issue, yes.
Changing class to a CommonDBRelation is probably out of the scope of a fix - I'm not sure we should do that kind of change in a bugfixes release.
About the remaining fails, I guess deletion should just be handled differently - the fix just reveals something that does not work.
|
Tests are broken, I do not yet know why. Proposed fix is probably incorrect, I put back as draft. |
The JS test for the dashboard joke failed :( |
3 participants
Also fix Twig display
closes #21488