Skip to content

Fix: make search options ID constant across profiles #593

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

Merged
cedric-anne merged 5 commits into from
Nov 18, 2022
Merged

Fix: make search options ID constant across profiles #593

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4b0e630
fix(container): make search options ID constant across profiles
btry Nov 16, 2022
f864c0b
Update inc/container.class.php
cedric-anne Nov 16, 2022
5bb99a0
Ensure SO have a stable ID
cedric-anne Nov 16, 2022
269e992
Migrate existing SO
cedric-anne Nov 16, 2022
992cf38
Use iterator
cedric-anne Nov 16, 2022
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
107 changes: 67 additions & 40 deletions inc/container.class.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1587,40 +1587,69 @@ public static function getAddSearchOptions($itemtype, $containers_id = false)

$opt = [];

$i = 76665;

// itemtype is stored in a JSON array, so entry is surrounded by double quotes
$search_string = json_encode($itemtype);
// Backslashes must be doubled in LIKE clause, according to MySQL documentation:
// > To search for \, specify it as \\\\; this is because the backslashes are stripped
// > once by the parser and again when the pattern match is made,
// > leaving a single backslash to be matched against.
$search_string = str_replace('\\', '\\\\', $search_string);

$query = "SELECT DISTINCT fields.id, fields.name, fields.label, fields.type, fields.is_readonly, fields.allowed_values,
containers.name as container_name, containers.label as container_label,
containers.itemtypes, containers.id as container_id, fields.id as field_id
FROM glpi_plugin_fields_containers containers";
$request = [
'SELECT' => [
'glpi_plugin_fields_fields.id AS field_id',
'glpi_plugin_fields_fields.name AS field_name',
'glpi_plugin_fields_fields.label AS field_label',
'glpi_plugin_fields_fields.type',
'glpi_plugin_fields_fields.is_readonly',
'glpi_plugin_fields_fields.allowed_values',
'glpi_plugin_fields_containers.id AS container_id',
'glpi_plugin_fields_containers.name AS container_name',
'glpi_plugin_fields_containers.label AS container_label',
(
Session::isCron()
? new QueryExpression(sprintf('%s AS %s', READ + CREATE, $DB->quoteName('right')))
: 'glpi_plugin_fields_profiles.right'
)
],
'DISTINCT' => true,
'FROM' => 'glpi_plugin_fields_fields',
'INNER JOIN' => [
'glpi_plugin_fields_containers' => [
'FKEY' => [
'glpi_plugin_fields_containers' => 'id',
'glpi_plugin_fields_fields' => 'plugin_fields_containers_id',
]
],
'glpi_plugin_fields_profiles' => [
'FKEY' => [
'glpi_plugin_fields_containers' => 'id',
'glpi_plugin_fields_profiles' => 'plugin_fields_containers_id',
]
],
],
'WHERE' => [
'glpi_plugin_fields_containers.is_active' => 1,
'glpi_plugin_fields_containers.itemtypes' => ['LIKE', '%' . $DB->escape($search_string) . '%'],
'glpi_plugin_fields_profiles.right' => ['>', 0],
'glpi_plugin_fields_fields.is_active' => 1,
['NOT' => ['glpi_plugin_fields_fields.type' => 'header']],
],
'ORDERBY' => [
'glpi_plugin_fields_fields.id',
],
];
if ($containers_id !== false) {
$request['WHERE'][] = ['glpi_plugin_fields_containers.id' => $containers_id];
}
if (!Session::isCron()) {
$query .= " INNER JOIN glpi_plugin_fields_profiles profiles
ON containers.id = profiles.plugin_fields_containers_id
AND profiles.right > 0
AND profiles.profiles_id = " . (int)$_SESSION['glpiactiveprofile']['id'];
}
$query .= " INNER JOIN glpi_plugin_fields_fields fields
ON containers.id = fields.plugin_fields_containers_id
AND containers.is_active = 1
WHERE containers.itemtypes LIKE '%" . $DB->escape($search_string) . "%'
AND fields.type != 'header'
ORDER BY fields.id ASC";
$res = $DB->query($query);
while ($data = $DB->fetchAssoc($res)) {
if ($containers_id !== false) {
// Filter by container (don't filter by SQL for have $i value with few containers for a itemtype)
if ($data['container_id'] != $containers_id) {
$i++;
continue;
}
}
$request['WHERE'][] = ['glpi_plugin_fields_profiles.profiles_id' => (int)$_SESSION['glpiactiveprofile']['id']];
}

$iterator = $DB->request($request);
foreach ($iterator as $data) {
$i = PluginFieldsField::SEARCH_OPTION_STARTING_INDEX + $data['field_id'];

$tablename = getTableForItemType(self::getClassname($itemtype, $data['container_name']));

//get translations
Expand All @@ -1634,15 +1663,15 @@ public static function getAddSearchOptions($itemtype, $containers_id = false)
$field = [
'itemtype' => PluginFieldsField::getType(),
'id' => $data['field_id'],
'label' => $data['label']
'label' => $data['field_label']
];
$data['label'] = PluginFieldsLabelTranslation::getLabelFor($field);
$data['field_label'] = PluginFieldsLabelTranslation::getLabelFor($field);

// Default SO params
$opt[$i]['table'] = $tablename;
$opt[$i]['field'] = $data['name'];
$opt[$i]['name'] = $data['container_label'] . " - " . $data['label'];
$opt[$i]['linkfield'] = $data['name'];
$opt[$i]['field'] = $data['field_name'];
$opt[$i]['name'] = $data['container_label'] . " - " . $data['field_label'];
$opt[$i]['linkfield'] = $data['field_name'];
$opt[$i]['joinparams']['jointype'] = "itemtype_item";
$opt[$i]['pfields_type'] = $data['type'];
if ($data['is_readonly']) {
Expand All @@ -1669,9 +1698,9 @@ public static function getAddSearchOptions($itemtype, $containers_id = false)

$dropdown_matches = [];
if ($data['type'] === "dropdown") {
$opt[$i]['table'] = 'glpi_plugin_fields_' . $data['name'] . 'dropdowns';
$opt[$i]['table'] = 'glpi_plugin_fields_' . $data['field_name'] . 'dropdowns';
$opt[$i]['field'] = 'completename';
$opt[$i]['linkfield'] = "plugin_fields_" . $data['name'] . "dropdowns_id";
$opt[$i]['linkfield'] = "plugin_fields_" . $data['field_name'] . "dropdowns_id";
$opt[$i]['datatype'] = "dropdown";

$opt[$i]['forcegroupby'] = true;
Expand All @@ -1685,7 +1714,7 @@ public static function getAddSearchOptions($itemtype, $containers_id = false)
) {
$opt[$i]['table'] = CommonDBTM::getTable($dropdown_matches['class']);
$opt[$i]['field'] = 'name';
$opt[$i]['linkfield'] = $data['name'];
$opt[$i]['linkfield'] = $data['field_name'];
$opt[$i]['right'] = 'all';
$opt[$i]['datatype'] = "dropdown";

Expand All @@ -1695,13 +1724,13 @@ public static function getAddSearchOptions($itemtype, $containers_id = false)
$opt[$i]['joinparams']['beforejoin']['table'] = $tablename;
$opt[$i]['joinparams']['beforejoin']['joinparams']['jointype'] = "itemtype_item";
} elseif ($data['type'] === "glpi_item") {
$itemtype_field = sprintf('itemtype_%s', $data['name']);
$items_id_field = sprintf('items_id_%s', $data['name']);
$itemtype_field = sprintf('itemtype_%s', $data['field_name']);
$items_id_field = sprintf('items_id_%s', $data['field_name']);

$opt[$i]['table'] = $tablename;
$opt[$i]['field'] = $itemtype_field;
$opt[$i]['linkfield'] = $itemtype_field;
$opt[$i]['name'] = $data['container_label'] . " - " . $data['label'] . ' - ' . _n('Associated item type', 'Associated item types', Session::getPluralNumber());
$opt[$i]['name'] = $data['container_label'] . " - " . $data['field_label'] . ' - ' . _n('Associated item type', 'Associated item types', Session::getPluralNumber());
$opt[$i]['datatype'] = 'itemtypename';
$opt[$i]['types'] = !empty($data['allowed_values']) ? json_decode($data['allowed_values']) : [];
$opt[$i]['additionalfields'] = ['itemtype'];
Expand All @@ -1713,14 +1742,12 @@ public static function getAddSearchOptions($itemtype, $containers_id = false)
$opt[$i]['table'] = $tablename;
$opt[$i]['field'] = $items_id_field;
$opt[$i]['linkfield'] = $items_id_field;
$opt[$i]['name'] = $data['container_label'] . " - " . $data['label'] . ' - ' . __('Associated item ID');
$opt[$i]['name'] = $data['container_label'] . " - " . $data['field_label'] . ' - ' . __('Associated item ID');
$opt[$i]['massiveaction'] = false;
$opt[$i]['joinparams']['jointype'] = 'itemtype_item';
$opt[$i]['datatype'] = 'text';
$opt[$i]['additionalfields'] = ['itemtype'];
}

$i++;
}

return $opt;
Expand Down
88 changes: 88 additions & 0 deletions inc/field.class.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ class PluginFieldsField extends CommonDBChild
{
use Glpi\Features\Clonable;

/**
* Starting index for search options.
* @var integer
*/
public const SEARCH_OPTION_STARTING_INDEX = 76665;

public static $itemtype = PluginFieldsContainer::class;
public static $items_id = 'plugin_fields_containers_id';

Expand Down Expand Up @@ -120,9 +126,91 @@ public static function install(Migration $migration, $version)
)
);

// 1.18.3 Make search options ID stable over time ad constant across profiles
if (Config::getConfigurationValue('plugin:fields', 'stable_search_options') !== 'yes') {
self::migrateToStableSO($migration);
$migration->addConfig(['stable_search_options' => 'yes'], 'plugin:fields');
}

return true;
}

/**
* Migrate search options ID stored in DB to their new stable ID.
*
* Prior to 1.18.3, search options ID were built using a simple increment and filtered using current profile rights,
* resulting in following behaviours:
* - when a container was activated/deactivated/removed, SO ID were potentially changed;
* - when a field was removed, SO ID were potentially changed;
* - in a sessionless context (e.g. CLI command/crontask), no SO were available;
* - when user added a SO in its display preference from a A profile, this SO was sometimes targetting a completely different field on a B profile.
* All of these behaviours were resulting in unstable display preferences and saved searches.
*
* Producing an exact mapping between previous unstable SO ID and new stable SO ID is almost impossible in many cases, due to
* previously described behaviours. Basically, we cannot know if the current SO ID in database is still correct
* and what were the profile rights when it was generated.
*
* @param Migration $migration
*/
private static function migrateToStableSO(Migration $migration): void
{
global $DB;

// Flatten itemtype list
$itemtypes = array_keys(array_merge([], ...array_values(PluginFieldsToolbox::getGlpiItemtypes())));

foreach ($itemtypes as $itemtype) {
// itemtype is stored in a JSON array, so entry is surrounded by double quotes
$search_string = json_encode($itemtype);
// Backslashes must be doubled in LIKE clause, according to MySQL documentation:
// > To search for \, specify it as \\\\; this is because the backslashes are stripped
// > once by the parser and again when the pattern match is made,
// > leaving a single backslash to be matched against.
$search_string = str_replace('\\', '\\\\', $search_string);

$fields = $DB->request(
[
'SELECT' => [
'glpi_plugin_fields_fields.id',
],
'FROM' => 'glpi_plugin_fields_fields',
'INNER JOIN' => [
'glpi_plugin_fields_containers' => [
'FKEY' => [
'glpi_plugin_fields_containers' => 'id',
'glpi_plugin_fields_fields' => 'plugin_fields_containers_id',
[
'AND' => [
'glpi_plugin_fields_containers.is_active' => 1,
]
]
]
],
],
'WHERE' => [
'glpi_plugin_fields_containers.itemtypes' => ['LIKE', '%' . $DB->escape($search_string) . '%'],
['NOT' => ['glpi_plugin_fields_fields.type' => 'header']],
],
'ORDERBY' => [
'glpi_plugin_fields_fields.id',
],
]
);

$i = PluginFieldsField::SEARCH_OPTION_STARTING_INDEX;

foreach ($fields as $field_data) {
$migration->changeSearchOption(
$itemtype,
$i,
PluginFieldsField::SEARCH_OPTION_STARTING_INDEX + $field_data['id']
);

$i++;
}
}
}

public static function uninstall()
{
global $DB;
Expand Down