Skip to content

Feat(Core): handle multiple configuration#169

Open
stonebuzz wants to merge 5 commits into
mainfrom
multi_sccm
Open

Feat(Core): handle multiple configuration#169
stonebuzz wants to merge 5 commits into
mainfrom
multi_sccm

Conversation

@stonebuzz
Copy link
Copy Markdown
Contributor

Checklist before requesting a review

Please delete options that are not relevant.

  • I have performed a self-review of my code.
  • I have added tests (when available) that prove my fix is effective or that my feature works.
  • I have updated the CHANGELOG with a short functional description of the fix or new feature.
  • This change requires a documentation update.

Description

Add the ability to handle multiple SCCM configurations.

Suggested replacement for #120

Screenshots (if appropriate):

@stonebuzz stonebuzz marked this pull request as draft May 19, 2026 09:57
@stonebuzz stonebuzz requested a review from Rom1-B May 19, 2026 10:00
@stonebuzz stonebuzz marked this pull request as ready for review May 19, 2026 10:00
@Rom1-B

This comment was marked as duplicate.

Rom1-B

This comment was marked as duplicate.

Copy link
Copy Markdown
Contributor

@Rom1-B Rom1-B left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase

@tolemac
Copy link
Copy Markdown
Contributor

tolemac commented May 20, 2026

Hi!

image

In the edit form the fields Name and comment are duplicated.

I'm currently running collect action from a big database, tell you how this end in a little.

@tolemac
Copy link
Copy Markdown
Contributor

tolemac commented May 20, 2026

I have too many devices in database ... I think I have to stop it ...

At the moment I'm looking the log, it is collecting devices, I have advised that It seems slower that before,
This is the log of this morning:

2026-05-20 08:00:01 [@SUInventario]
Getting devices ... 
2026-05-20 08:00:17 [@SUInventario]
getDevices OK 
2026-05-20 08:00:17 [@SUInventario]
Generate XML start : 274 files
2026-05-20 08:00:18 [@SUInventario]
Collect OK for device - 16784352 
2026-05-20 08:00:18 [@SUInventario]
Collect OK for device - 16784363 
2026-05-20 08:00:18 [@SUInventario]
Collect OK for device - 16784374 
2026-05-20 08:00:19 [@SUInventario]
Collect OK for device - 16784381 
2026-05-20 08:00:19 [@SUInventario]
Collect OK for device - 16784383 
2026-05-20 08:00:20 [@SUInventario]
Collect OK for device - 16784385 
2026-05-20 08:00:20 [@SUInventario]
Collect OK for device - 16784474 
2026-05-20 08:00:21 [@SUInventario]
Collect OK for device - 16784513 
2026-05-20 08:00:22 [@SUInventario]
Collect OK for device - 16784519 
2026-05-20 08:00:22 [@SUInventario]
Collect OK for device - 16784520 
2026-05-20 08:00:22 [@SUInventario]
Collect OK for device - 16784522 
2026-05-20 08:00:23 [@SUInventario]
Collect OK for device - 16784525 
2026-05-20 08:00:23 [@SUInventario]

You can see how to collect several devices by second, however now:

2026-05-20 13:25:44 [473@SUInventario]
[Config 1] getDevices OK - 8280 files
2026-05-20 13:25:47 [473@SUInventario]
[Config 1] Collect OK for device - 16777658
2026-05-20 13:25:49 [473@SUInventario]
[Config 1] Collect OK for device - 16777924
2026-05-20 13:25:52 [473@SUInventario]
[Config 1] Collect OK for device - 16784352
2026-05-20 13:25:55 [473@SUInventario]
[Config 1] Collect OK for device - 16784363
2026-05-20 13:25:57 [473@SUInventario]
[Config 1] Collect OK for device - 16784374
2026-05-20 13:26:00 [473@SUInventario]
[Config 1] Collect OK for device - 16784381
2026-05-20 13:26:02 [473@SUInventario]
[Config 1] Collect OK for device - 16784383
2026-05-20 13:26:05 [473@SUInventario]
[Config 1] Collect OK for device - 16784385
2026-05-20 13:26:07 [473@SUInventario]
[Config 1] Collect OK for device - 16784474
2026-05-20 13:26:10 [473@SUInventario]
[Config 1] Collect OK for device - 16784513
2026-05-20 13:26:13 [473@SUInventario]
[Config 1] Collect OK for device - 16784519

Each device take more than 2 or 3 seconds ...

I'm not sure why ...

@tolemac
Copy link
Copy Markdown
Contributor

tolemac commented May 20, 2026

I have stop it ... only 319 devices on 15 minutes ...

I can't run it for all devices in database, there are thousands, I need collection name filter...

@stonebuzz
Copy link
Copy Markdown
Contributor Author

@tolemac

I fixed the issue with the duplicate fields in the form.

Regarding the collection timing, I’d prefer to handle that in a dedicated PR.

For now, let’s finalize this one first, then implement the collection mechanism. After that, we’ll evaluate whether the collection times are acceptable and optimize them if needed.

Best regards

@stonebuzz stonebuzz self-assigned this May 20, 2026
@stonebuzz stonebuzz requested a review from Rom1-B May 20, 2026 12:17
@tolemac
Copy link
Copy Markdown
Contributor

tolemac commented May 20, 2026

@tolemac

I fixed the issue with the duplicate fields in the form.

Regarding the collection timing, I’d prefer to handle that in a dedicated PR.

For now, let’s finalize this one first, then implement the collection mechanism. After that, we’ll evaluate whether the collection times are acceptable and optimize them if needed.

Best regards

Totally agree!

Comment thread front/config.php
Comment on lines +36 to +50
Session::checkRight('config', UPDATE);

Html::header(
PluginSccmConfig::getTypeName(Session::getPluralNumber()),
'',
'config',
'PluginSccmMenu',
);

$config = new PluginSccmConfig();
if ($config->canView()) {
Search::show(PluginSccmConfig::class);
} else {
throw new AccessDeniedHttpException();
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe:

Suggested change
Session::checkRight('config', UPDATE);
Html::header(
PluginSccmConfig::getTypeName(Session::getPluralNumber()),
'',
'config',
'PluginSccmMenu',
);
$config = new PluginSccmConfig();
if ($config->canView()) {
Search::show(PluginSccmConfig::class);
} else {
throw new AccessDeniedHttpException();
}
Session::checkRight(PluginSccmConfig::$rightname, UPDATE);
Session::checkRight(PluginSccmConfig::$rightname, READ);
Html::header(
PluginSccmConfig::getTypeName(Session::getPluralNumber()),
'',
'config',
'PluginSccmMenu',
);
Search::show(PluginSccmConfig::class);

or create checkRightsAnd() in core

Comment thread front/config.form.php

$sccm_db = new PluginSccmSccmdb();
if ($sccm_db->connect()) {
if ($sccm_db->connect($_POST['id'])) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if ($sccm_db->connect($_POST['id'])) {
if ($sccm_db->connect((int) $_POST['id'])) {

Comment thread hook.php
mkdir($xml_root . '1', 0755, true);
}

foreach (glob($xml_root . '*.ocs') ?: [] as $file) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

safe\glob() never returns false

Suggested change
foreach (glob($xml_root . '*.ocs') ?: [] as $file) {
foreach (glob($xml_root . '*.ocs') as $file) {

Comment thread hook.php
mkdir(GLPI_PLUGIN_DOC_DIR . '/sccm/xml');
}

// Migrate existing flat XML files to per-config subdirectory (config id=1)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The XML file migration occurs before the Migration object is created and before the PluginSccmConfig::install() method is called. If the database migration fails, the files will already have been moved and it will no longer be possible to undo the changes. Perhaps the order should be reversed: DB first, then files?

Comment thread inc/config.class.php
$this->initForm($ID, $options);

$password = (new GLPIKey())->decrypt($this->fields['sccmdb_password'] ?? '');
$url = ($this->fields['inventory_server_url'] ?: "Ex : " . $CFG_GLPI['url_base']) . '/front/inventory.php';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$url = ($this->fields['inventory_server_url'] ?: "Ex : " . $CFG_GLPI['url_base']) . '/front/inventory.php';
$url = ($this->fields['inventory_server_url'] ?: __('Example:') . ' ' . $CFG_GLPI['url_base']) . '/front/inventory.php';

Comment thread inc/config.class.php
return true;
}

public static function isIdAutoIncrement()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public static function isIdAutoIncrement()
public static function isIdAutoIncrement(): bool

Comment thread inc/config.class.php
$fields = $DB->listFields('glpi_plugin_sccm_configs');
if (isset($fields['id'])) {
$fieldData = $fields['id'];
$extra = is_object($fieldData) ? ($fieldData->Extra ?? $fieldData->extra ?? '') : ($fieldData['Extra'] ?? $fieldData['extra'] ?? '');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe:

Suggested change
$extra = is_object($fieldData) ? ($fieldData->Extra ?? $fieldData->extra ?? '') : ($fieldData['Extra'] ?? $fieldData['extra'] ?? '');
$extra = $fields['id']['Extra'] ?? '';

{{ fields.checkboxField(
'use_auth_ntlm',
item.fields['use_auth_ntlm'],
__('Use NLTM authentication', 'sccm')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
__('Use NLTM authentication', 'sccm')
__('Use NTLM authentication', 'sccm')

{{ fields.textField(
'auth_info',
item.fields['auth_info'],
__('Value for spécific authentication', 'sccm')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
__('Value for spécific authentication', 'sccm')
__('Value for specific authentication', 'sccm')

{% if not item.isNewItem() %}
{{ fields.nullField() }}
{% set btn_test %}
<form method="post" action="{{ path('/plugins/sccm/front/config.form.php') }}">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we not already in a form?!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants