Add old_id and new_id col in glpi_logs table

[RomainLvr]

Checklist before requesting a review

Please delete options that are not relevant.

• I have read the CONTRIBUTING document.
• I have performed a self-review of my code.
• I have added tests that prove my fix is effective or that my feature works.
• This change requires a documentation update.

Description

• It fixes Feat - User computer history #19140
• Here is a brief description of what this PR does

Add two new_id and old_id columns to the glpi_logs table to optimize table searches

[Copilot]

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

[cedric-anne]

This condition has been added to fix the following error that appeared during the migration from 0.85.x to 0.90.0:

MySQL query error: Unknown column 'old_id' in 'INSERT INTO' (1054) in SQL query "INSERT INTO `glpi_logs` (`items_id`, `itemtype`, `itemtype_link`, `linked_action`, `user_name`, `date_mod`, `id_search_option`, `old_value`, `new_value`, `old_id`, `new_id`) VALUES ('1', 'Config', '', '0', '', '2025-03-07 15:13:08', '1', 'palette ', 'auror', NULL, NULL)".

IMHO, migrations are not supposed to log anything. The following patch would fix this error, but it is probably not the only one required.

diff --git a/install/migrations/update_0.85.x_to_0.90.0.php b/install/migrations/update_0.85.x_to_0.90.0.php
index 112c8cc6eb..55c619b3f2 100644
--- a/install/migrations/update_0.85.x_to_0.90.0.php
+++ b/install/migrations/update_0.85.x_to_0.90.0.php
@@ -74,16 +74,18 @@ function update085xto0900()
     }
 
    // Add Color selector
-    Config::setConfigurationValues('core', ['palette' => 'auror']);
+    $migration->addConfig(['palette' => 'auror']);
     $migration->addField("glpi_users", "palette", "char(20) DEFAULT NULL");
 
    // add layout config
-    Config::setConfigurationValues('core', ['layout' => 'lefttab']);
+    $migration->addConfig(['layout' => 'lefttab']);
     $migration->addField("glpi_users", "layout", "char(20) DEFAULT NULL");
 
    // add timeline config
-    Config::setConfigurationValues('core', ['ticket_timeline' => 1]);
-    Config::setConfigurationValues('core', ['ticket_timeline_keep_replaced_tabs' => 0]);
+    $migration->addConfig([
+        'ticket_timeline' => 1,
+        'ticket_timeline_keep_replaced_tabs' => 0,
+    ]);
     $migration->addField("glpi_users", "ticket_timeline", "tinyint DEFAULT NULL");
     $migration->addField("glpi_users", "ticket_timeline_keep_replaced_tabs", "tinyint DEFAULT NULL");

I am rarely comfortable with having to patch old migrations, but I do not like having these kinds of conditions either. @trasher What is your opinion about this ?

[trasher]

Hum... I do not like to change old migrations neither, but I do agree it should not log anything.
Since alternative solution seems tricky; and the migration fix is quite light; I'd be OK for this change.

[RomainLvr]

This patch does not solve the problem :(

[cedric-anne]

We are working on a patch that would remove usages of CommonDBTM::add()/CommonDBTM::update() in migrations. I would like to see if we can achieve that in order to remove this condition.

[cedric-anne]

We are working on a patch that would remove usages of CommonDBTM::add()/CommonDBTM::update() in migrations. I would like to see if we can achieve that in order to remove this condition.